Release 6.4.5 (#7319)

* White-listing of telemetry params (#7262)

* Drop support for adding telemetry by tagging

This was not used, and would make development of future feature more difficult.

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Introduce white-listing for params of telemetry

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Fix linting

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Make misconfigured telemetry for function parameters log the error, and not blow up fatally

Co-authored-by: Janne Savolainen <janne.savolainen@live.fi>

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

---------

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

* Release 6.4.5

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

---------

Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>

lerna.json

@@ -4,7 +4,7 @@
   "packages": [
     "packages/*"
   ],
-  "version": "6.4.4",
+  "version": "6.4.5",
   "npmClient": "yarn",
   "npmClientArgs": [
     "--network-timeout=100000"

packages/core/package.json

@@ -3,7 +3,7 @@
   "productName": "",
   "description": "Lens Desktop Core",
   "homepage": "https://github.com/lensapp/lens",
-  "version": "6.4.4",
+  "version": "6.4.5",
   "repository": {
     "type": "git",
     "url": "git+https://github.com/lensapp/lens.git"

packages/core/src/common/log-error.injectable.ts

@@ -8,6 +8,7 @@ import loggerInjectable from "./logger.injectable";
 const logErrorInjectable = getInjectable({
   id: "log-error",
   instantiate: (di) => di.inject(loggerInjectable).error,
+  decorable: false,
 });
 
 export default logErrorInjectable;

packages/core/src/common/logger.injectable.ts

@@ -26,6 +26,8 @@ const loggerInjectable = getInjectable({
       silly: (message, ...data) => baseLogger.silly(message, ...data),
     };
   },
+
+  decorable: false,
 });
 
 export default loggerInjectable;

packages/core/src/features/telemetry/emit-telemetry-from-specific-function-calls.test.ts

@@ -8,6 +8,7 @@ import { getInjectable } from "@ogre-tools/injectable";
 import { getDiForUnitTesting } from "../../renderer/getDiForUnitTesting";
 import telemetryWhiteListForFunctionsInjectable from "./renderer/telemetry-white-list-for-functions.injectable";
 import emitEventInjectable from "../../common/app-event-bus/emit-event.injectable";
+import logErrorInjectable from "../../common/log-error.injectable";
 
 describe("emit-telemetry-from-specific-function-calls", () => {
   let di: DiContainer;
@@ -22,72 +23,158 @@ describe("emit-telemetry-from-specific-function-calls", () => {
     beforeEach(() => {
       di.override(telemetryWhiteListForFunctionsInjectable, () => [
         "some-white-listed-function",
+
+        {
+          id: "some-white-listed-function-with-white-listed-argument",
+          getParams: (irrelevantArg, arg) => ({ someParam: arg }),
+        },
+
+        {
+          id: "some-white-listed-function-with-bad-config",
+
+          getParams: () => {
+            throw new Error("some-error-from-bad-configuration");
+          },
+        },
       ]);
 
       emitEventMock = jest.fn();
       di.override(emitEventInjectable, () => emitEventMock);
     });
 
-    describe("given instances of white-listed, non-white-listed and tagged functions", () => {
+    describe("given instances of white-listed and non-white-listed functions", () => {
       let whiteListedFunctionMock: jest.Mock;
       let nonWhiteListedFunctionMock: jest.Mock;
-      let taggedFunctionMock: jest.Mock;
+      let whiteListedFunction: jest.Mock;
-      let injectedWhiteListedFunction: jest.Mock;
+      let whiteListedFunctionWithArgument: jest.Mock;
-      let injectedNonWhiteListedFunction: jest.Mock;
+      let whiteListedFunctionWithFaultyConfig: jest.Mock;
-      let injectedTaggedFunction: jest.Mock;
+      let nonWhiteListedFunction: jest.Mock;
+      let logErrorMock: jest.Mock;
 
       beforeEach(() => {
         whiteListedFunctionMock = jest.fn();
         nonWhiteListedFunctionMock = jest.fn();
-        taggedFunctionMock = jest.fn();
+        logErrorMock = jest.fn();
 
         const whiteListedInjectable = getInjectable({
           id: "some-white-listed-function",
           instantiate: () => whiteListedFunctionMock,
         });
 
+        const whiteListedInjectableWithArgument = getInjectable({
+          id: "some-white-listed-function-with-white-listed-argument",
+          instantiate: () => whiteListedFunctionMock,
+        });
+
+        const whiteListedInjectableWithBadConfig = getInjectable({
+          id: "some-white-listed-function-with-bad-config",
+          instantiate: () => whiteListedFunctionMock,
+        });
+
         const nonWhiteListedInjectable = getInjectable({
           id: "some-non-white-listed-function",
           instantiate: () => nonWhiteListedFunctionMock,
         });
 
-        const taggedInjectable = getInjectable({
-          id: "some-tagged-function",
-          instantiate: () => taggedFunctionMock,
-          tags: ["emit-telemetry"],
-        });
-
         runInAction(() => {
-          di.register(whiteListedInjectable);
+          di.register(
-          di.register(nonWhiteListedInjectable);
+            whiteListedInjectable,
-          di.register(taggedInjectable);
+            whiteListedInjectableWithArgument,
+            whiteListedInjectableWithBadConfig,
+            nonWhiteListedInjectable,
+          );
         });
 
-        injectedWhiteListedFunction = di.inject(whiteListedInjectable);
+        di.override(logErrorInjectable, () => logErrorMock);
-        injectedNonWhiteListedFunction = di.inject(nonWhiteListedInjectable);
+
-        injectedTaggedFunction = di.inject(taggedInjectable);
+        whiteListedFunction = di.inject(whiteListedInjectable);
+
+        whiteListedFunctionWithArgument = di.inject(
+          whiteListedInjectableWithArgument,
+        );
+
+        whiteListedFunctionWithFaultyConfig = di.inject(
+          whiteListedInjectableWithBadConfig,
+        );
+
+        nonWhiteListedFunction = di.inject(nonWhiteListedInjectable);
       });
 
       it("telemetry is not emitted yet", () => {
         expect(emitEventMock).not.toHaveBeenCalled();
       });
 
-      describe("when the white-listed function is called", () => {
+      it("doesn't log errors, at least yet", () => {
+        expect(logErrorMock).not.toHaveBeenCalled();
+      });
+
+      describe("when a normal white-listed function is called with arguments", () => {
         beforeEach(() => {
-          injectedWhiteListedFunction("some-arg", "some-other-arg");
+          whiteListedFunction("some-arg", "some-other-arg");
         });
 
-        it("telemetry is emitted in event bus", () => {
+        it("telemetry is emitted in event bus without the arguments", () => {
           expect(emitEventMock).toHaveBeenCalledWith({
             destination: "auto-capture",
             action: "telemetry-from-business-action",
             name: "some-white-listed-function",
-            params: { args: ["some-arg", "some-other-arg"] },
           });
         });
       });
 
-      describe("when the white-listed function is called with MobX reactive content", () => {
+      describe("when a white-listed function with a white-listed argument is called with arguments", () => {
+        beforeEach(() => {
+          whiteListedFunctionWithArgument("some-arg", "some-other-arg");
+        });
+
+        it("telemetry is emitted in event bus with the arguments as params", () => {
+          expect(emitEventMock).toHaveBeenCalledWith({
+            action: "telemetry-from-business-action",
+            destination: "auto-capture",
+            name: "some-white-listed-function-with-white-listed-argument",
+            params: { someParam: "some-other-arg" },
+          });
+        });
+      });
+
+      describe("when a white-listed function with a white-listed argument is called without arguments", () => {
+        beforeEach(() => {
+          whiteListedFunctionWithArgument();
+        });
+
+        it("telemetry is emitted in event bus without params", () => {
+          expect(emitEventMock).toHaveBeenCalledWith({
+            action: "telemetry-from-business-action",
+            destination: "auto-capture",
+            name: "some-white-listed-function-with-white-listed-argument",
+            params: { someParam: undefined },
+          });
+        });
+      });
+
+      describe("given a faulty configuration, when a white-listed function is called", () => {
+        beforeEach(() => {
+          whiteListedFunctionWithFaultyConfig();
+        });
+
+        it("telemetry is still emitted in event bus, but with params indicating bad configuration, ", () => {
+          expect(emitEventMock).toHaveBeenCalledWith({
+            action: "telemetry-from-business-action",
+            destination: "auto-capture",
+            name: "some-white-listed-function-with-bad-config",
+            params: { error: "Tried to produce params for telemetry, but getParams() threw an error" },
+          });
+        });
+
+        it("logs error", () => {
+          expect(logErrorMock).toHaveBeenCalledWith(
+            'Tried to produce params for telemetry of "some-white-listed-function-with-bad-config", but getParams() threw an error',
+            expect.objectContaining({ message: "some-error-from-bad-configuration" }),
+          );
+        });
+      });
+
+      describe("when a white-listed function with a white-listed argument is called with MobX reactive content", () => {
         beforeEach(() => {
           const someComputedProperty = computed(() => "some-computed-value");
 
@@ -96,22 +183,23 @@ describe("emit-telemetry-from-specific-function-calls", () => {
             someComputedProperty,
           };
 
-          injectedWhiteListedFunction(someObservable);
+          whiteListedFunctionWithArgument(
+            "irrelevant-argument",
+            someObservable,
+          );
         });
 
         it("telemetry is emitted in event bus without MobX internals or computeds", () => {
           expect(emitEventMock).toHaveBeenCalledWith({
             destination: "auto-capture",
             action: "telemetry-from-business-action",
-            name: "some-white-listed-function",
+            name: "some-white-listed-function-with-white-listed-argument",
 
             params: {
-              args: [
+              someParam: {
-                {
+                someStaticProperty: "some-static-value",
-                  someStaticProperty: "some-static-value",
+                someComputedProperty: "some-computed-value",
-                  someComputedProperty: "some-computed-value",
+              },
-                },
-              ],
             },
           });
         });
@@ -119,28 +207,13 @@ describe("emit-telemetry-from-specific-function-calls", () => {
 
       describe("when the non-white-listed function is called", () => {
         beforeEach(() => {
-          injectedNonWhiteListedFunction();
+          nonWhiteListedFunction();
         });
 
         it("telemetry is not emitted", () => {
           expect(emitEventMock).not.toHaveBeenCalled();
         });
       });
-
-      describe("when the tagged, but not white-listed function is called", () => {
-        beforeEach(() => {
-          injectedTaggedFunction("some-arg", "some-other-arg");
-        });
-
-        it("telemetry is emitted in event bus", () => {
-          expect(emitEventMock).toHaveBeenCalledWith({
-            destination: "auto-capture",
-            action: "telemetry-from-business-action",
-            name: "some-tagged-function",
-            params: { args: ["some-arg", "some-other-arg"] },
-          });
-        });
-      });
     });
   });
 });

packages/core/src/features/telemetry/renderer/emit-telemetry.injectable.ts

@@ -4,7 +4,7 @@
  */
 import { getInjectable } from "@ogre-tools/injectable";
 import emitEventInjectable from "../../../common/app-event-bus/emit-event.injectable";
-import { toJS, observable } from "mobx";
+import { observable, toJS } from "mobx";
 
 const emitTelemetryInjectable = getInjectable({
   id: "emit-telemetry",
@@ -12,12 +12,12 @@ const emitTelemetryInjectable = getInjectable({
   instantiate: (di) => {
     const emitEvent = di.inject(emitEventInjectable);
 
-    return ({ action, args }: { action: string; args: any[] }) => {
+    return ({ action, params }: { action: string; params?: object }) => {
       emitEvent({
         destination: "auto-capture",
         action: "telemetry-from-business-action",
         name: action,
-        params: { args: toJS(observable(args)) },
+        ...(params ? { params: toJS(observable(params)) } : {}),
       });
     };
   },

packages/core/src/features/telemetry/renderer/telemetry-decorator.injectable.ts

@@ -2,33 +2,34 @@
  * Copyright (c) OpenLens Authors. All rights reserved.
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
-import type {
+import type { DiContainerForInjection } from "@ogre-tools/injectable";
-  DiContainerForInjection,
-  Injectable,
-} from "@ogre-tools/injectable";
 
 import {
-  lifecycleEnum,
   getInjectable,
   instantiationDecoratorToken,
+  lifecycleEnum,
 } from "@ogre-tools/injectable";
-import assert from "assert";
 
+import assert from "assert";
 import { isFunction } from "lodash/fp";
 import emitTelemetryInjectable from "./emit-telemetry.injectable";
+
+import type { WhiteListItem } from "./telemetry-white-list-for-functions.injectable";
 import telemetryWhiteListForFunctionsInjectable from "./telemetry-white-list-for-functions.injectable";
+import logErrorInjectable from "../../../common/log-error.injectable";
 
 const telemetryDecoratorInjectable = getInjectable({
   id: "telemetry-decorator",
 
   instantiate: (diForDecorator) => {
     const emitTelemetry = diForDecorator.inject(emitTelemetryInjectable);
+    const logError = diForDecorator.inject(logErrorInjectable);
 
     const whiteList = diForDecorator.inject(
       telemetryWhiteListForFunctionsInjectable,
     );
 
-    const shouldEmitTelemetry = shouldEmitTelemetryFor(whiteList);
+    const whiteListMap = getWhiteListMap(whiteList);
 
     return {
       decorate:
@@ -42,8 +43,31 @@ const telemetryDecoratorInjectable = getInjectable({
 
                 assert(currentContext);
 
-                if (shouldEmitTelemetry(currentContext.injectable)) {
+                const whiteListed = whiteListMap.get(
-                  emitTelemetry({ action: currentContext.injectable.id, args });
+                  currentContext.injectable.id,
+                );
+
+                if (whiteListed) {
+                  let params;
+
+                  try {
+                    params = whiteListed.getParams(...args);
+                  } catch (e) {
+                    params = {
+                      error:
+                      "Tried to produce params for telemetry, but getParams() threw an error",
+                    };
+
+                    logError(
+                      `Tried to produce params for telemetry of "${currentContext.injectable.id}", but getParams() threw an error`,
+                      e,
+                    );
+                  }
+
+                  emitTelemetry({
+                    action: currentContext.injectable.id,
+                    params,
+                  });
                 }
 
                 return instance(...args);
@@ -61,9 +85,23 @@ const telemetryDecoratorInjectable = getInjectable({
   injectionToken: instantiationDecoratorToken,
 });
 
-const shouldEmitTelemetryFor =
+const getWhiteListMap = (whiteList: WhiteListItem[]) =>
-  (whiteList: string[]) => (injectable: Injectable<any, any, any>) =>
+  new Map(
-    injectable.tags?.includes("emit-telemetry") ||
+    whiteList.map((item) =>
-    whiteList.includes(injectable.id);
+      typeof item === "string"
+        ? [
+          item,
+          {
+            getParams: () => undefined,
+          },
+        ]
+        : [
+          item.id,
+          {
+            getParams: item.getParams,
+          },
+        ],
+    ),
+  );
 
 export default telemetryDecoratorInjectable;

packages/core/src/features/telemetry/renderer/telemetry-white-list-for-functions.injectable.ts

@@ -3,6 +3,7 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 import { getInjectable } from "@ogre-tools/injectable";
+import type { AppEvent } from "../../../common/app-event-bus/event-bus";
 
 const navigateTo = [
   "navigate-to-preference-tab-id",
@@ -88,21 +89,19 @@ const extensions = [
   "uninstall-extension",
 ];
 
-const externalActions = [
+const externalActions = ["open-link-in-browser"];
-  "open-link-in-browser",
-];
 
-const uiInteraction = [
+const uiInteraction = ["show-details"];
-  "show-details",
-];
 
-const terminal = [
+const terminal = ["create-terminal-tab"];
-  "create-terminal-tab",
+
-];
+export type WhiteListItem =
+  | string
+  | { id: string; getParams: (...args: unknown[]) => AppEvent["params"] };
 
 const telemetryWhiteListForFunctionsInjectable = getInjectable({
   id: "telemetry-white-list-for-functions",
-  instantiate: () => [
+  instantiate: (): WhiteListItem[] => [
     ...navigateTo,
     ...helmInjectableIds,
     ...kubeConfigActions,

packages/core/src/main/logger/console-transport.injectable.ts

@@ -30,6 +30,7 @@ const consoleLoggerTransportInjectable = getInjectable({
     ),
   }),
   injectionToken: loggerTransportInjectionToken,
+  decorable: false,
 });
 
 export default consoleLoggerTransportInjectable;

packages/core/src/main/logger/file-transport.injectable.ts

@@ -23,6 +23,7 @@ const fileLoggerTranportInjectable = getInjectable({
     tailable: true,
   }),
   injectionToken: loggerTransportInjectionToken,
+  decorable: false,
 });
 
 export default fileLoggerTranportInjectable;

packages/core/src/renderer/logger/browser-transport.injectable.ts

@@ -10,6 +10,7 @@ const browserLoggerTransportInjectable = getInjectable({
   id: "browser-logger-transport",
   instantiate: () => new BrowserConsole(),
   injectionToken: loggerTransportInjectionToken,
+  decorable: false,
 });
 
 export default browserLoggerTransportInjectable;

packages/extension-api/package.json

@@ -2,7 +2,7 @@
   "name": "@k8slens/extensions",
   "productName": "OpenLens extensions",
   "description": "OpenLens - Open Source Kubernetes IDE: extensions",
-  "version": "6.4.4",
+  "version": "6.4.5",
   "copyright": "© 2022 OpenLens Authors",
   "license": "MIT",
   "main": "dist/extension-api.js",
@@ -26,7 +26,7 @@
     "prepare:dev": "yarn run build"
   },
   "dependencies": {
-    "@k8slens/core": "^6.4.4"
+    "@k8slens/core": "^6.4.5"
   },
   "devDependencies": {
     "@types/node": "^16.18.6",

packages/open-lens/package.json

@@ -4,7 +4,7 @@
   "productName": "OpenLens",
   "description": "OpenLens - Open Source IDE for Kubernetes",
   "homepage": "https://github.com/lensapp/lens",
-  "version": "6.4.4",
+  "version": "6.4.5",
   "repository": {
     "type": "git",
     "url": "git+https://github.com/lensapp/lens.git"
@@ -192,7 +192,7 @@
     }
   },
   "dependencies": {
-    "@k8slens/core": "^6.4.4",
+    "@k8slens/core": "^6.4.5",
     "@k8slens/ensure-binaries": "^6.4.0-beta.16",
     "@k8slens/generate-tray-icons": "^6.4.0-beta.16",
     "@ogre-tools/fp": "^12.0.1",