From 36ed4ce2917c6ff7438f165a83f6e3907d8b9837 Mon Sep 17 00:00:00 2001 From: Esteban Beltran Date: Mon, 2 Oct 2023 16:15:37 +0200 Subject: [PATCH] Plugins: Fix regexes in metrics tracking and remove plugin guess logic (#75720) --- .../plugins/loader/packageMetrics.test.ts | 42 --------------- .../features/plugins/loader/packageMetrics.ts | 51 ++----------------- 2 files changed, 3 insertions(+), 90 deletions(-) diff --git a/public/app/features/plugins/loader/packageMetrics.test.ts b/public/app/features/plugins/loader/packageMetrics.test.ts index f687515d8f3..dd69e6ba6e6 100644 --- a/public/app/features/plugins/loader/packageMetrics.test.ts +++ b/public/app/features/plugins/loader/packageMetrics.test.ts @@ -49,7 +49,6 @@ describe('trackPackageUsage', () => { key: 'foo', parent: 'your-package', packageName: 'your-package', - guessedPluginName: '', }); expect(result).toEqual(obj); }); @@ -72,13 +71,11 @@ describe('trackPackageUsage', () => { key: 'foo2', parent: 'your-package', packageName: 'your-package', - guessedPluginName: '', }); expect(logInfoMock).toHaveBeenCalledWith(`Plugin using your-package.foo2.bar`, { key: 'bar', parent: 'your-package.foo2', packageName: 'your-package', - guessedPluginName: '', }); expect(result.foo2).toEqual(obj.foo2); @@ -115,14 +112,12 @@ describe('trackPackageUsage', () => { key: 'foo3', parent: 'your-package', packageName: 'your-package', - guessedPluginName: '', }); mockUsage(result2.foo3.bar); expect(logInfoMock).toHaveBeenCalledWith(`Plugin using your-package.foo3.bar`, { key: 'bar', parent: 'your-package.foo3', packageName: 'your-package', - guessedPluginName: '', }); expect(result1.foo3).toEqual(obj.foo3); @@ -159,43 +154,6 @@ describe('trackPackageUsage', () => { }); }); - it('should guess the plugin name from the stacktrace and urls', () => { - const mockErrorConstructor = jest.fn().mockImplementation(() => { - return { - stack: `Error - at eval (eval at get (http://localhost:3000/public/build/app.38735bee027ded74d65e.js:167859:11), :1:1) - at Object.get (http://localhost:3000/public/build/app.38735bee027ded74d65e.js:167859:11) - at eval (http://localhost:3000/public/plugins/alexanderzobnin-zabbix-app/panel-triggers/module.js?_cache=1695283550582:3:2582) - at eval (http://localhost:3000/public/plugins/alexanderzobnin-zabbix-app/panel-triggers/module.js?_cache=1695283550582:159:22081) - at Object.eval (http://localhost:3000/public/plugins/alexanderzobnin-zabbix-app/panel-triggers/module.js?_cache=1695283550582:159:22087) - at Object.execute (http://localhost:3000/public/build/app.38735bee027ded74d65e.js:529405:37) - at doExec (http://localhost:3000/public/build/app.38735bee027ded74d65e.js:529955:32) - at postOrderExec (http://localhost:3000/public/build/app.38735bee027ded74d65e.js:529951:12) - at http://localhost:3000/public/build/app.38735bee027ded74d65e.js:529899:14 - at async http://localhost:3000/public/build/app.38735bee027ded74d65e.js:166261:16`, - }; - }); - - const errorSpy = jest.spyOn(global, 'Error').mockImplementation(mockErrorConstructor); - - const obj = { - lord: 'me', - }; - - const result = trackPackageUsage(obj, 'your-package'); - mockUsage(result.lord); - - expect(logInfoMock).toHaveBeenCalledTimes(1); - expect(logInfoMock).toHaveBeenLastCalledWith(`Plugin using your-package.lord`, { - key: 'lord', - parent: 'your-package', - packageName: 'your-package', - guessedPluginName: 'alexanderzobnin-zabbix-app', - }); - - errorSpy.mockRestore(); - }); - it('Should skip tracking if document.currentScript is not null', () => { // Save the original value of the attribute const originalCurrentScript = document.currentScript; diff --git a/public/app/features/plugins/loader/packageMetrics.ts b/public/app/features/plugins/loader/packageMetrics.ts index cbf3d495ee8..690edd938b7 100644 --- a/public/app/features/plugins/loader/packageMetrics.ts +++ b/public/app/features/plugins/loader/packageMetrics.ts @@ -3,46 +3,6 @@ import { logInfo } from '@grafana/runtime'; const cachedMetricProxies = new WeakMap(); const trackedKeys: Record = {}; -let pluginNameFromUrlRegex = /plugins\/([^/]*)\/.*?module\.js/i; - -/** - * This function attempts to determine the plugin name by - * analyzing the stack trace. It achieves this by generating - * an error object and accessing its stack property, - * which typically includes the script URL. - * - * Note that when inside an async function of any kind, the - * stack trace is somewhat lost and the plugin name cannot - * be determined most of the times. - * - * It assumes that the plugin ID is part of the URL, - * although this cannot be guaranteed. - * - * Please note that this function is specifically designed - * for plugins loaded with systemjs. - * - * It is important to treat the information provided by - * this function as a "best guess" and not rely on it - * for any business logic. - */ -function guessPluginNameFromStack(): string | undefined { - try { - const errorStack = new Error().stack; - if (errorStack?.includes('systemJSPrototype')) { - return undefined; - } - if (errorStack && errorStack.includes('module.js')) { - let match = errorStack.match(pluginNameFromUrlRegex); - if (match && match[1]) { - return match[1]; - } - } - } catch (e) { - return undefined; - } - return undefined; -} - function createMetricsProxy(obj: T, parentName: string, packageName: string): T { const handler: ProxyHandler = { get(target, key) { @@ -55,11 +15,10 @@ function createMetricsProxy(obj: T, parentName: string, packag // that we don't want to track key.toString() !== '__useDefault' ) { - const pluginName = guessPluginNameFromStack() ?? ''; const accessPath = `${parentName}.${String(key)}`; // we want to report API usage per-plugin when possible - const cacheKey = `${pluginName}:${accessPath}`; + const cacheKey = `${accessPath}`; if (!trackedKeys[cacheKey]) { trackedKeys[cacheKey] = true; @@ -69,17 +28,13 @@ function createMetricsProxy(obj: T, parentName: string, packag key: String(key), parent: parentName, packageName: packageName, - guessedPluginName: pluginName, }); } } - // typescript will not trust the key is a key of target, but given this is a proxy handler - // it is guarantee that `key` is a key of `target` so we can type assert to make types work - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - const value = target[key as keyof T]; + const value = Reflect.get(target, key); - if (value !== null && typeof value === 'object') { + if (value !== null && typeof value === 'object' && !(value instanceof RegExp)) { if (!cachedMetricProxies.has(value)) { cachedMetricProxies.set(value, createMetricsProxy(value, `${parentName}.${String(key)}`, packageName)); }