Plugin Extension: improves mutation logs (#107370)

* Plugin Extension: improves  mutation logs

* chore: removes feature toggle

* chore: rename function

packages/grafana-data/src/types/featureToggles.gen.ts

@@ -976,10 +976,6 @@ export interface FeatureToggles {
   */
   alertingBulkActionsInUI?: boolean;
   /**
-  * Use proxy-based read-only objects for plugin extensions instead of deep cloning
-  */
-  extensionsReadOnlyProxy?: boolean;
-  /**
   * Registers AuthZ /apis endpoint
   */
   kubernetesAuthzApis?: boolean;

pkg/services/featuremgmt/registry.go

@@ -1675,15 +1675,6 @@ var (
 			HideFromDocs:      true,
 			Expression:        "true", // enabled by default
 		},
-		{
-			Name:              "extensionsReadOnlyProxy",
-			Description:       "Use proxy-based read-only objects for plugin extensions instead of deep cloning",
-			Stage:             FeatureStageExperimental,
-			Owner:             grafanaPluginsPlatformSquad,
-			HideFromAdminPage: true,
-			HideFromDocs:      true,
-			FrontendOnly:      true,
-		},
 		{
 			Name:              "kubernetesAuthzApis",
 			Description:       "Registers AuthZ /apis endpoint",

pkg/services/featuremgmt/toggles_gen.csv

@@ -219,7 +219,6 @@ multiTenantFrontend,experimental,@grafana/grafana-frontend-platform,false,false,
 alertingListViewV2PreviewToggle,privatePreview,@grafana/alerting-squad,false,false,true
 alertRuleUseFiredAtForStartsAt,experimental,@grafana/alerting-squad,false,false,false
 alertingBulkActionsInUI,GA,@grafana/alerting-squad,false,false,true
-extensionsReadOnlyProxy,experimental,@grafana/plugins-platform-backend,false,false,true
 kubernetesAuthzApis,experimental,@grafana/identity-access-team,false,false,false
 restoreDashboards,experimental,@grafana/grafana-frontend-platform,false,false,false
 skipTokenRotationIfRecent,privatePreview,@grafana/identity-access-team,false,false,false

pkg/services/featuremgmt/toggles_gen.go

@@ -887,10 +887,6 @@ const (
 	// Enables the alerting bulk actions in the UI
 	FlagAlertingBulkActionsInUI = "alertingBulkActionsInUI"
 
-	// FlagExtensionsReadOnlyProxy
-	// Use proxy-based read-only objects for plugin extensions instead of deep cloning
-	FlagExtensionsReadOnlyProxy = "extensionsReadOnlyProxy"
-
 	// FlagKubernetesAuthzApis
 	// Registers AuthZ /apis endpoint
 	FlagKubernetesAuthzApis = "kubernetesAuthzApis"

pkg/services/featuremgmt/toggles_gen.json

@@ -1110,7 +1110,8 @@
       "metadata": {
         "name": "extensionsReadOnlyProxy",
         "resourceVersion": "1750434297879",
-        "creationTimestamp": "2025-05-06T04:55:23Z"
+        "creationTimestamp": "2025-05-06T04:55:23Z",
+        "deletionTimestamp": "2025-06-30T08:24:11Z"
       },
       "spec": {
         "description": "Use proxy-based read-only objects for plugin extensions instead of deep cloning",

public/app/features/datasources/components/DataSourcePluginSettings.tsx

@@ -1,7 +1,7 @@
 import { createElement, PureComponent } from 'react';
 
 import { DataSourcePluginMeta, DataSourceSettings } from '@grafana/data';
-import { readOnlyCopy } from 'app/features/plugins/extensions/utils';
+import { writableProxy } from 'app/features/plugins/extensions/utils';
 
 import { GenericDataSourcePlugin } from '../types';
 
@@ -34,7 +34,7 @@ export class DataSourcePluginSettings extends PureComponent<Props> {
       <div>
         {plugin.components.ConfigEditor &&
           createElement(plugin.components.ConfigEditor, {
-            options: readOnlyCopy(dataSource),
+            options: writableProxy(dataSource),
             onOptionsChange: this.onModelChanged,
           })}
       </div>

public/app/features/plugins/extensions/usePluginComponent.test.tsx

@@ -380,8 +380,8 @@ describe('usePluginComponent()', () => {
     // Should not throw an error if it mutates the props
     expect(() => render(Component && <Component {...originalProps} override />)).not.toThrow();
 
-    // Should log a warning
-    expect(log.warning).toHaveBeenCalledWith('Attempted to mutate object property "c"', {
+    // Should log an error in dev mode
+    expect(log.error).toHaveBeenCalledWith('Attempted to mutate object property "c"', {
       stack: expect.any(String),
     });
   });

public/app/features/plugins/extensions/usePluginComponents.test.tsx

@@ -266,7 +266,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"`, {
+    expect(log.error).toHaveBeenCalledWith(`Attempted to mutate object property "foo4"`, {
       stack: expect.any(String),
     });
 

public/app/features/plugins/extensions/utils.test.tsx

@@ -21,8 +21,7 @@ import {
   getAppPluginDependencies,
   getExtensionPointPluginMeta,
   getMutationObserverProxy,
-  readOnlyCopy,
-  isReadOnlyProxy,
+  writableProxy,
   isMutationObserverProxy,
 } from './utils';
 
@@ -401,7 +400,7 @@ describe('Plugin Extensions / Utils', () => {
       expect(proxy.a).toBe('b');
     });
 
-    it('should be possible to set new values, but logs a warning', () => {
+    it('should be possible to set new values, but logs a debug message', () => {
       const obj: { a: string; b?: string } = { a: 'a' };
       const proxy = getMutationObserverProxy(obj);
 
@@ -412,7 +411,7 @@ describe('Plugin Extensions / Utils', () => {
         });
       }).not.toThrow();
 
-      expect(log.warning).toHaveBeenCalledWith(`Attempted to define object property "b"`, {
+      expect(log.debug).toHaveBeenCalledWith(`Attempted to define object property "b"`, {
         stack: expect.any(String),
       });
 
@@ -440,12 +439,11 @@ describe('Plugin Extensions / Utils', () => {
     });
   });
 
-  describe('readOnlyCopy()', () => {
+  describe('writableProxy()', () => {
     const originalEnv = config.buildInfo.env;
 
     beforeEach(() => {
       jest.spyOn(console, 'warn').mockImplementation();
-      config.featureToggles.extensionsReadOnlyProxy = false;
     });
 
     afterEach(() => {
@@ -454,34 +452,19 @@ describe('Plugin Extensions / Utils', () => {
     });
 
     it('should return the same value for primitive types', () => {
-      expect(readOnlyCopy(1)).toBe(1);
-      expect(readOnlyCopy('a')).toBe('a');
-      expect(readOnlyCopy(true)).toBe(true);
-      expect(readOnlyCopy(false)).toBe(false);
-      expect(readOnlyCopy(null)).toBe(null);
-      expect(readOnlyCopy(undefined)).toBe(undefined);
-    });
-
-    it('should return a read-only proxy of the original object if the feature flag is enabled', () => {
-      config.featureToggles.extensionsReadOnlyProxy = true;
-
-      const obj = { a: 'a' };
-      const copy = readOnlyCopy(obj);
-
-      expect(copy).not.toBe(obj);
-      expect(copy.a).toBe('a');
-      expect(isReadOnlyProxy(copy)).toBe(true);
-      expect(() => {
-        copy.a = 'b';
-      }).toThrow(TypeError);
+      expect(writableProxy(1)).toBe(1);
+      expect(writableProxy('a')).toBe('a');
+      expect(writableProxy(true)).toBe(true);
+      expect(writableProxy(false)).toBe(false);
+      expect(writableProxy(null)).toBe(null);
+      expect(writableProxy(undefined)).toBe(undefined);
     });
 
     it('should return a writable deep-copy of the original object in dev mode', () => {
-      config.featureToggles.extensionsReadOnlyProxy = false;
       config.buildInfo.env = 'development';
 
       const obj = { a: 'a' };
-      const copy = readOnlyCopy(obj);
+      const copy = writableProxy(obj);
 
       expect(copy).not.toBe(obj);
       expect(copy.a).toBe('a');
@@ -498,11 +481,10 @@ describe('Plugin Extensions / Utils', () => {
     });
 
     it('should return a writable deep-copy of the original object in production mode', () => {
-      config.featureToggles.extensionsReadOnlyProxy = false;
       config.buildInfo.env = 'production';
 
       const obj = { a: 'a' };
-      const copy = readOnlyCopy(obj);
+      const copy = writableProxy(obj);
 
       expect(copy).not.toBe(obj);
       expect(copy.a).toBe('a');
@@ -519,11 +501,10 @@ describe('Plugin Extensions / Utils', () => {
     });
 
     it('should allow freezing the object in production mode', () => {
-      config.featureToggles.extensionsReadOnlyProxy = false;
       config.buildInfo.env = 'production';
 
       const obj = { a: 'a', b: { c: 'c' } };
-      const copy = readOnlyCopy(obj);
+      const copy = writableProxy(obj);
 
       expect(() => {
         Object.freeze(copy);
@@ -534,7 +515,7 @@ describe('Plugin Extensions / Utils', () => {
       expect(Object.isFrozen(copy.b)).toBe(true);
       expect(copy.b).toEqual({ c: 'c' });
 
-      expect(log.warning).toHaveBeenCalledWith(`Attempted to define object property "a"`, {
+      expect(log.debug).toHaveBeenCalledWith(`Attempted to define object property "a"`, {
         stack: expect.any(String),
       });
     });
@@ -687,7 +668,7 @@ describe('Plugin Extensions / Utils', () => {
       expect(screen.getByText('Version: 1.0.0')).toBeVisible();
     });
 
-    it('should not be possible to mutate the props in development mode, but it logs a warning', async () => {
+    it('should not be possible to mutate the props in development mode, but it logs an error', async () => {
       config.buildInfo.env = 'development';
       const pluginId = 'grafana-worldmap-panel';
       const Component = wrapWithPluginContext(pluginId, ExampleComponent, log);
@@ -700,8 +681,8 @@ describe('Plugin Extensions / Utils', () => {
       expect(await screen.findByText('Hello Grafana!')).toBeVisible();
 
       // Logs a warning
-      expect(log.warning).toHaveBeenCalledTimes(1);
-      expect(log.warning).toHaveBeenCalledWith(`Attempted to mutate object property "c"`, {
+      expect(log.error).toHaveBeenCalledTimes(1);
+      expect(log.error).toHaveBeenCalledWith(`Attempted to mutate object property "c"`, {
         stack: expect.any(String),
       });
 

public/app/features/plugins/extensions/utils.tsx

@@ -85,7 +85,7 @@ export const wrapWithPluginContext = <T,>(pluginId: string, Component: React.Com
 
     return (
       <PluginContextProvider meta={pluginMeta}>
-        <Component {...readOnlyCopy(props, log)} />
+        <Component {...writableProxy(props, log)} />
       </PluginContextProvider>
     );
   };
@@ -230,24 +230,27 @@ export function getMutationObserverProxy<T extends object>(obj: T, _log: Extensi
   }
 
   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) {
-      _log.warning(`Attempted to delete object property "${String(prop)}"`, {
+      logFunction(`Attempted to delete object property "${String(prop)}"`, {
         stack: new Error().stack ?? '',
       });
       Reflect.deleteProperty(target, prop);
       return true;
     },
     defineProperty(target, prop, descriptor) {
-      _log.warning(`Attempted to define object property "${String(prop)}"`, {
+      // 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)}"`, {
         stack: new Error().stack ?? '',
       });
       Reflect.defineProperty(target, prop, descriptor);
       return true;
     },
     set(target, prop, newValue) {
-      _log.warning(`Attempted to mutate object property "${String(prop)}"`, {
+      logFunction(`Attempted to mutate object property "${String(prop)}"`, {
         stack: new Error().stack ?? '',
       });
       Reflect.set(target, prop, newValue);
@@ -285,16 +288,12 @@ export function getMutationObserverProxy<T extends object>(obj: T, _log: Extensi
   });
 }
 
-export function readOnlyCopy<T>(value: T, _log: ExtensionsLog = log): T {
+export function writableProxy<T>(value: T, _log: ExtensionsLog = log): T {
   // Primitive types are read-only by default
   if (!value || typeof value !== 'object') {
     return value;
   }
 
-  if (config.featureToggles.extensionsReadOnlyProxy) {
-    return getReadOnlyProxy(value);
-  }
-
   // 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);
 }