Plugins: adds plugin version to mutation logs (#108057)

* Plugins: adds plugin version to mutation logs

* chore: updates after PR feedback
This commit is contained in:
Hugo Häggmark
2025-07-14 11:52:48 +02:00
committed by GitHub
parent 75a6aa7838
commit 4260dea554
4 changed files with 54 additions and 32 deletions

View File

@ -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),
}

View File

@ -267,7 +267,7 @@ describe('usePluginComponents()', () => {
// Should also render the component if it wants to change the props
expect(() => render(<Component foo={originalFoo} override />)).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(<Component foo={originalFoo} override />)).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),
}

View File

@ -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),
}

View File

@ -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<T extends object>(obj: T, options?: ProxyOptions): T {
@ -268,31 +270,40 @@ export function getMutationObserverProxy<T extends object>(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<T extends object>(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<T extends object>(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<T>(value: T, options?: ProxyOptions): T {
@ -344,10 +356,10 @@ export function writableProxy<T>(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<string | number | symbol, unknown> {