From 4260dea554ae4378f36fe8b80f9e14dc9204a532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 14 Jul 2025 11:52:48 +0200 Subject: [PATCH] Plugins: adds plugin version to mutation logs (#108057) * Plugins: adds plugin version to mutation logs * chore: updates after PR feedback --- .../extensions/usePluginComponent.test.tsx | 4 +- .../extensions/usePluginComponents.test.tsx | 4 +- .../plugins/extensions/utils.test.tsx | 40 ++++++++++++------- .../app/features/plugins/extensions/utils.tsx | 38 ++++++++++++------ 4 files changed, 54 insertions(+), 32 deletions(-) diff --git a/public/app/features/plugins/extensions/usePluginComponent.test.tsx b/public/app/features/plugins/extensions/usePluginComponent.test.tsx index 9ce151c2200..51341e2051f 100644 --- a/public/app/features/plugins/extensions/usePluginComponent.test.tsx +++ b/public/app/features/plugins/extensions/usePluginComponent.test.tsx @@ -382,7 +382,7 @@ describe('usePluginComponent()', () => { // Should log an error in dev mode expect(log.error).toHaveBeenCalledWith( - 'Attempted to mutate object property "c" from extension with id myorg-extensions-app', + 'Attempted to mutate object property "c" from extension with id myorg-extensions-app and version unknown', { stack: expect.any(String), } @@ -438,7 +438,7 @@ describe('usePluginComponent()', () => { // Should log a warning expect(log.warning).toHaveBeenCalledWith( - 'Attempted to mutate object property "c" from extension with id myorg-extensions-app', + 'Attempted to mutate object property "c" from extension with id myorg-extensions-app and version unknown', { stack: expect.any(String), } diff --git a/public/app/features/plugins/extensions/usePluginComponents.test.tsx b/public/app/features/plugins/extensions/usePluginComponents.test.tsx index 7b55daa2800..f40955c3f54 100644 --- a/public/app/features/plugins/extensions/usePluginComponents.test.tsx +++ b/public/app/features/plugins/extensions/usePluginComponents.test.tsx @@ -267,7 +267,7 @@ describe('usePluginComponents()', () => { // Should also render the component if it wants to change the props expect(() => render()).not.toThrow(); expect(log.error).toHaveBeenCalledWith( - `Attempted to mutate object property "foo4" from extension with id myorg-extensions-app`, + `Attempted to mutate object property "foo4" from extension with id myorg-extensions-app and version unknown`, { stack: expect.any(String), } @@ -331,7 +331,7 @@ describe('usePluginComponents()', () => { // Should also render the component if it wants to change the props expect(() => render()).not.toThrow(); expect(log.warning).toHaveBeenCalledWith( - `Attempted to mutate object property "foo4" from extension with id myorg-extensions-app`, + `Attempted to mutate object property "foo4" from extension with id myorg-extensions-app and version unknown`, { stack: expect.any(String), } diff --git a/public/app/features/plugins/extensions/utils.test.tsx b/public/app/features/plugins/extensions/utils.test.tsx index a99df507d1b..33da958ae89 100644 --- a/public/app/features/plugins/extensions/utils.test.tsx +++ b/public/app/features/plugins/extensions/utils.test.tsx @@ -399,14 +399,17 @@ describe('Plugin Extensions / Utils', () => { }); it('should be possible to modify values in proxied object, but logs an error', () => { - const proxy = getMutationObserverProxy({ a: 'a' }, { pluginId: 'myorg-cool-datasource', source: 'datasource' }); + const proxy = getMutationObserverProxy( + { a: 'a' }, + { pluginId: 'myorg-cool-datasource', source: 'datasource', pluginVersion: '1.2.3' } + ); expect(() => { proxy.a = 'b'; }).not.toThrow(); expect(log.error).toHaveBeenCalledWith( - `Attempted to mutate object property "a" from datasource with id myorg-cool-datasource`, + `Attempted to mutate object property "a" from datasource with id myorg-cool-datasource and version 1.2.3`, { stack: expect.any(String), } @@ -427,7 +430,7 @@ describe('Plugin Extensions / Utils', () => { }).not.toThrow(); expect(log.debug).toHaveBeenCalledWith( - `Attempted to define object property "b" from extension with id myorg-cool-extension`, + `Attempted to define object property "b" from extension with id myorg-cool-extension and version unknown`, { stack: expect.any(String), } @@ -450,7 +453,7 @@ describe('Plugin Extensions / Utils', () => { }).not.toThrow(); expect(log.error).toHaveBeenCalledWith( - `Attempted to delete object property "c" from extension with id unknown`, + `Attempted to delete object property "c" from extension with id unknown and version unknown`, { stack: expect.any(String), } @@ -473,7 +476,7 @@ describe('Plugin Extensions / Utils', () => { }).not.toThrow(); expect(log.warning).toHaveBeenCalledWith( - `Attempted to mutate object property "a" from datasource with id myorg-cool-datasource`, + `Attempted to mutate object property "a" from datasource with id myorg-cool-datasource and version unknown`, { stack: expect.any(String), } @@ -494,7 +497,7 @@ describe('Plugin Extensions / Utils', () => { }).not.toThrow(); expect(log.debug).toHaveBeenCalledWith( - `Attempted to define object property "b" from extension with id myorg-cool-extension`, + `Attempted to define object property "b" from extension with id myorg-cool-extension and version unknown`, { stack: expect.any(String), } @@ -517,7 +520,7 @@ describe('Plugin Extensions / Utils', () => { }).not.toThrow(); expect(log.warning).toHaveBeenCalledWith( - `Attempted to delete object property "c" from extension with id unknown`, + `Attempted to delete object property "c" from extension with id unknown and version unknown`, { stack: expect.any(String), } @@ -548,7 +551,11 @@ describe('Plugin Extensions / Utils', () => { config.buildInfo.env = 'development'; const obj = { a: 'a' }; - const copy = writableProxy(obj, { source: 'datasource', pluginId: 'myorg-cool-datasource' }); + const copy = writableProxy(obj, { + source: 'datasource', + pluginId: 'myorg-cool-datasource', + pluginVersion: '1.2.3', + }); expect(copy).not.toBe(obj); expect(copy.a).toBe('a'); @@ -558,7 +565,7 @@ describe('Plugin Extensions / Utils', () => { }).not.toThrow(); expect(log.error).toHaveBeenCalledWith( - `Attempted to mutate object property "a" from datasource with id myorg-cool-datasource`, + `Attempted to mutate object property "a" from datasource with id myorg-cool-datasource and version 1.2.3`, { stack: expect.any(String), } @@ -581,7 +588,7 @@ describe('Plugin Extensions / Utils', () => { }).not.toThrow(); expect(log.warning).toHaveBeenCalledWith( - `Attempted to mutate object property "a" from datasource with id myorg-cool-datasource`, + `Attempted to mutate object property "a" from datasource with id myorg-cool-datasource and version unknown`, { stack: expect.any(String), } @@ -605,9 +612,12 @@ describe('Plugin Extensions / Utils', () => { expect(Object.isFrozen(copy.b)).toBe(true); expect(copy.b).toEqual({ c: 'c' }); - expect(log.debug).toHaveBeenCalledWith(`Attempted to define object property "a" from extension with id unknown`, { - stack: expect.any(String), - }); + expect(log.debug).toHaveBeenCalledWith( + `Attempted to define object property "a" from extension with id unknown and version unknown`, + { + stack: expect.any(String), + } + ); }); }); @@ -893,7 +903,7 @@ describe('Plugin Extensions / Utils', () => { // Logs a warning expect(log.error).toHaveBeenCalledTimes(1); expect(log.error).toHaveBeenCalledWith( - `Attempted to mutate object property "c" from extension with id grafana-worldmap-panel`, + `Attempted to mutate object property "c" from extension with id grafana-worldmap-panel and version unknown`, { stack: expect.any(String), } @@ -921,7 +931,7 @@ describe('Plugin Extensions / Utils', () => { // Logs a warning expect(log.warning).toHaveBeenCalledTimes(1); expect(log.warning).toHaveBeenCalledWith( - `Attempted to mutate object property "c" from extension with id grafana-worldmap-panel`, + `Attempted to mutate object property "c" from extension with id grafana-worldmap-panel and version unknown`, { stack: expect.any(String), } diff --git a/public/app/features/plugins/extensions/utils.tsx b/public/app/features/plugins/extensions/utils.tsx index 63a2d2bfc83..db17481a165 100644 --- a/public/app/features/plugins/extensions/utils.tsx +++ b/public/app/features/plugins/extensions/utils.tsx @@ -251,6 +251,7 @@ interface ProxyOptions { log?: ExtensionsLog; source?: MutationSource; pluginId?: string; + pluginVersion?: string; } /** @@ -261,6 +262,7 @@ interface ProxyOptions { * @param options.log The logger to use * @param options.source The source of the mutation * @param options.pluginId The id of the plugin that is mutating the object + * @param options.pluginVersion The version of the plugin that is mutating the object * @returns A new proxy object that logs any attempted mutation to the original object */ export function getMutationObserverProxy(obj: T, options?: ProxyOptions): T { @@ -268,31 +270,40 @@ export function getMutationObserverProxy(obj: T, options?: Pro return obj; } - const { log = baseLog, source = 'extension', pluginId = 'unknown' } = options ?? {}; + const { log = baseLog, source = 'extension', pluginId = 'unknown', pluginVersion = 'unknown' } = options ?? {}; const cache = new WeakMap(); const logFunction = isGrafanaDevMode() ? log.error.bind(log) : log.warning.bind(log); // should show error during local development return new Proxy(obj, { deleteProperty(target, prop) { - logFunction(`Attempted to delete object property "${String(prop)}" from ${source} with id ${pluginId}`, { - stack: new Error().stack ?? '', - }); + logFunction( + `Attempted to delete object property "${String(prop)}" from ${source} with id ${pluginId} and version ${pluginVersion}`, + { + stack: new Error().stack ?? '', + } + ); Reflect.deleteProperty(target, prop); return true; }, defineProperty(target, prop, descriptor) { // because immer (used by RTK) calls Object.isFrozen and Object.freeze we know that defineProperty will be called // behind the scenes as well so we only log message with debug level to minimize the noise and false positives - log.debug(`Attempted to define object property "${String(prop)}" from ${source} with id ${pluginId}`, { - stack: new Error().stack ?? '', - }); + log.debug( + `Attempted to define object property "${String(prop)}" from ${source} with id ${pluginId} and version ${pluginVersion}`, + { + stack: new Error().stack ?? '', + } + ); Reflect.defineProperty(target, prop, descriptor); return true; }, set(target, prop, newValue) { - logFunction(`Attempted to mutate object property "${String(prop)}" from ${source} with id ${pluginId}`, { - stack: new Error().stack ?? '', - }); + logFunction( + `Attempted to mutate object property "${String(prop)}" from ${source} with id ${pluginId} and version ${pluginVersion}`, + { + stack: new Error().stack ?? '', + } + ); Reflect.set(target, prop, newValue); return true; }, @@ -318,7 +329,7 @@ export function getMutationObserverProxy(obj: T, options?: Pro if (isObject(value) || isArray(value)) { if (!cache.has(value)) { - cache.set(value, getMutationObserverProxy(value, { log, source, pluginId })); + cache.set(value, getMutationObserverProxy(value, { log, source, pluginId, pluginVersion })); } return cache.get(value); } @@ -336,6 +347,7 @@ export function getMutationObserverProxy(obj: T, options?: Pro * @param options.log The logger to use * @param options.source The source of the mutation * @param options.pluginId The id of the plugin that is mutating the object + * @param options.pluginVersion The version of the plugin that is mutating the object * @returns A new proxy object that logs any attempted mutation to the original object */ export function writableProxy(value: T, options?: ProxyOptions): T { @@ -344,10 +356,10 @@ export function writableProxy(value: T, options?: ProxyOptions): T { return value; } - const { log = baseLog, source = 'extension', pluginId = 'unknown' } = options ?? {}; + const { log = baseLog, source = 'extension', pluginId = 'unknown', pluginVersion = 'unknown' } = options ?? {}; // Default: we return a proxy of a deep-cloned version of the original object, which logs warnings when mutation is attempted - return getMutationObserverProxy(cloneDeep(value), { log, pluginId, source }); + return getMutationObserverProxy(cloneDeep(value), { log, pluginId, pluginVersion, source }); } function isRecord(value: unknown): value is Record {