SQL Datasources: Update Max Connection and Max Idle Connection Defaults to 100 and add auto mode (#65834)

* Update connection configuration for SQL datasources

* Working auto state for connection numbers

* Add migration

* Use defaults from constants file

* Remove dead code

* Add tests and restructure useMigrateDatabaseField

* Update function names

* Update docs

* Make sure we don't continually issue updates

* Update docs

* Use onOptionsChnage in ConnectionLimits

* Update docs

* Clean up docs

* Update migration

* Fix default values in docs

* Fix spacing issue

* Fix test

* Update default values for SQL connections

* Include consts

* Allow override for default SQL datasource connection parameters

* Fix linter errors

* Remove extra @ts-ignore

* Centralize logic for default values

* Remove debugging

* Remove unecessary function

* Update configuration docs

* minor suggested change

* Fix comment misspelling

* Remove unecessary default setting code

* Update docs to indicate that code was included for backport version

* Remove dead code

---------

Co-authored-by: lwandz13 <larissa.wandzura@grafana.com>

public/app/features/plugins/sql/components/configuration/ConnectionLimits.tsx

@@ -1,27 +1,86 @@
 import React from 'react';
 
-import { FieldSet, InlineField } from '@grafana/ui';
+import { DataSourceSettings } from '@grafana/data';
+import { FieldSet, InlineField, InlineFieldRow, InlineSwitch } from '@grafana/ui';
 import { NumberInput } from 'app/core/components/OptionsUI/NumberInput';
 
-import { SQLConnectionLimits } from '../../types';
+import { SQLConnectionDefaults } from '../../constants';
+import { SQLConnectionLimits, SQLOptions } from '../../types';
 
 interface Props<T> {
-  onPropertyChanged: (property: keyof T, value?: number) => void;
+  onOptionsChange: Function;
+  options: DataSourceSettings<SQLOptions>;
   labelWidth: number;
-  jsonData: SQLConnectionLimits;
 }
 
 export const ConnectionLimits = <T extends SQLConnectionLimits>(props: Props<T>) => {
-  const { onPropertyChanged, labelWidth, jsonData } = props;
+  const { onOptionsChange, options, labelWidth } = props;
+  const jsonData = options.jsonData;
+  const autoIdle = jsonData.maxIdleConnsAuto !== undefined ? jsonData.maxIdleConnsAuto : false;
 
+  // Update JSON data with new values
+  const updateJsonData = (values: {}) => {
+    const newOpts = {
+      ...options,
+      jsonData: {
+        ...jsonData,
+        ...values,
+      },
+    };
+
+    return onOptionsChange(newOpts);
+  };
+
+  // For the case of idle connections and connection lifetime
+  // use a shared function to update respective properties
   const onJSONDataNumberChanged = (property: keyof SQLConnectionLimits) => {
     return (number?: number) => {
-      if (onPropertyChanged) {
-        onPropertyChanged(property, number);
-      }
+      updateJsonData({ property: number });
     };
   };
 
+  // When the maximum number of connections is changed
+  // see if we have the automatic idle option enabled
+  const onMaxConnectionsChanged = (number?: number) => {
+    if (autoIdle && number) {
+      updateJsonData({
+        maxOpenConns: number,
+        maxIdleConns: number,
+      });
+    } else if (number !== undefined) {
+      updateJsonData({
+        maxOpenConns: number,
+      });
+    }
+  };
+
+  // Update auto idle setting when control is toggled
+  // and set minimum idle connections if automatic
+  // is selected
+  const onConnectionIdleAutoChanged = () => {
+    let idleConns = undefined;
+    let maxConns = undefined;
+
+    // If the maximum number of open connections is undefined
+    // and we're setting auto idle then set the default amount
+    // otherwise take the numeric amount and get the value from that
+    if (!autoIdle) {
+      if (jsonData.maxOpenConns !== undefined) {
+        maxConns = jsonData.maxOpenConns;
+        idleConns = jsonData.maxOpenConns;
+      } else {
+        maxConns = SQLConnectionDefaults.MAX_CONNS;
+        idleConns = SQLConnectionDefaults.MAX_CONNS;
+      }
+    }
+
+    updateJsonData({
+      maxIdleConnsAuto: !autoIdle,
+      maxIdleConns: idleConns,
+      maxOpenConns: maxConns,
+    });
+  };
+
   return (
     <FieldSet label="Connection limits">
       <InlineField
@@ -36,29 +95,42 @@ export const ConnectionLimits = <T extends SQLConnectionLimits>(props: Props<T>)
         labelWidth={labelWidth}
         label="Max open"
       >
-        <NumberInput
-          placeholder="unlimited"
-          value={jsonData.maxOpenConns}
-          onChange={onJSONDataNumberChanged('maxOpenConns')}
-        ></NumberInput>
-      </InlineField>
-      <InlineField
-        tooltip={
-          <span>
-            The maximum number of connections in the idle connection pool.If <i>Max open connections</i> is greater than
-            0 but less than the <i>Max idle connections</i>, then the <i>Max idle connections</i> will be reduced to
-            match the <i>Max open connections</i> limit. If set to 0, no idle connections are retained.
-          </span>
-        }
-        labelWidth={labelWidth}
-        label="Max idle"
-      >
-        <NumberInput
-          placeholder="2"
-          value={jsonData.maxIdleConns}
-          onChange={onJSONDataNumberChanged('maxIdleConns')}
-        ></NumberInput>
+        <NumberInput placeholder="unlimited" value={jsonData.maxOpenConns} onChange={onMaxConnectionsChanged} />
       </InlineField>
+      <InlineFieldRow>
+        <InlineField
+          tooltip={
+            <span>
+              The maximum number of connections in the idle connection pool.If <i>Max open connections</i> is greater
+              than 0 but less than the <i>Max idle connections</i>, then the <i>Max idle connections</i> will be reduced
+              to match the <i>Max open connections</i> limit. If set to 0, no idle connections are retained.
+            </span>
+          }
+          labelWidth={labelWidth}
+          label="Max idle"
+        >
+          <NumberInput
+            placeholder="2"
+            value={jsonData.maxIdleConns}
+            onChange={onJSONDataNumberChanged('maxIdleConns')}
+            width={8}
+            fieldDisabled={autoIdle}
+          />
+        </InlineField>
+        <InlineField
+          label="Auto"
+          labelWidth={8}
+          tooltip={
+            <span>
+              If enabled, automatically set the number of <i>Maximum idle connections</i> to the same value as
+              <i> Max open connections</i>. If the number of maximum open connections is not set it will be set to the
+              default ({SQLConnectionDefaults.MAX_CONNS}).
+            </span>
+          }
+        >
+          <InlineSwitch value={autoIdle} onChange={onConnectionIdleAutoChanged} />
+        </InlineField>
+      </InlineFieldRow>
       <InlineField
         tooltip="The maximum amount of time in seconds a connection may be reused. If set to 0, connections are reused forever."
         labelWidth={labelWidth}

public/app/features/plugins/sql/components/configuration/useMigrateDatabaseField.ts

@@ -1,25 +0,0 @@
-import { useEffect } from 'react';
-
-import { DataSourceJsonData, DataSourcePluginOptionsEditorProps } from '@grafana/data';
-import { logDebug } from '@grafana/runtime';
-
-import { SQLOptions } from '../../types';
-
-/**
- * Moves the database field from the options object to jsonData.database and empties the database field.
- */
-export function useMigrateDatabaseField<T extends DataSourceJsonData = SQLOptions, S = {}>({
-  onOptionsChange,
-  options,
-}: DataSourcePluginOptionsEditorProps<T, S>) {
-  useEffect(() => {
-    if (options.database) {
-      logDebug(`Migrating from options.database with value ${options.database} for ${options.name}`);
-      onOptionsChange({
-        ...options,
-        database: '',
-        jsonData: { ...options.jsonData, database: options.database },
-      });
-    }
-  }, [onOptionsChange, options]);
-}

public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.test.ts

@@ -0,0 +1,71 @@
+import { renderHook } from '@testing-library/react-hooks';
+
+import { DataSourceSettings } from '@grafana/data';
+
+import { SQLConnectionDefaults } from '../../constants';
+import { SQLOptions } from '../../types';
+
+import { useMigrateDatabaseFields } from './useMigrateDatabaseFields';
+
+describe('Database Field Migration', () => {
+  let defaultProps = {
+    options: {
+      database: 'testDatabase',
+      id: 1,
+      uid: 'unique-id',
+      orgId: 1,
+      name: 'Datasource Name',
+      type: 'postgres',
+      typeName: 'Postgres',
+      typeLogoUrl: 'http://example.com/logo.png',
+      access: 'access',
+      url: 'http://example.com',
+      user: 'user',
+      basicAuth: true,
+      basicAuthUser: 'user',
+      isDefault: false,
+      secureJsonFields: {},
+      readOnly: false,
+      withCredentials: false,
+      jsonData: {
+        tlsAuth: false,
+        tlsAuthWithCACert: false,
+        timezone: 'America/Chicago',
+        tlsSkipVerify: false,
+        user: 'user',
+      },
+    },
+  };
+  it('should migrate the old database field to be included in jsonData', () => {
+    const props = {
+      ...defaultProps,
+      onOptionsChange: (options: DataSourceSettings) => {
+        const jsonData = options.jsonData as SQLOptions;
+        expect(options.database).toBe('');
+        expect(jsonData.database).toBe('testDatabase');
+      },
+    };
+
+    // @ts-ignore Ignore this line as it's expected that
+    // the database object will not be in necessary (most current) state
+    const { rerender, result } = renderHook(() => useMigrateDatabaseFields(props));
+    rerender();
+  });
+
+  it('adds default max connection, max idle connection, and auto idle values when not detected', () => {
+    const props = {
+      ...defaultProps,
+      onOptionsChange: (options: DataSourceSettings) => {
+        const jsonData = options.jsonData as SQLOptions;
+        expect(jsonData.maxOpenConns).toBe(SQLConnectionDefaults.MAX_CONNS);
+        expect(jsonData.maxIdleConns).toBe(Math.ceil(SQLConnectionDefaults.MAX_CONNS));
+        expect(jsonData.maxIdleConnsAuto).toBe(true);
+      },
+    };
+
+    // @ts-ignore Ignore this line as it's expected that
+    // the database object will not be in the expected (most current) state
+    const { rerender, result } = renderHook(() => useMigrateDatabaseFields(props));
+    rerender();
+  });
+});

public/app/features/plugins/sql/components/configuration/useMigrateDatabaseFields.ts

@@ -0,0 +1,63 @@
+import { useEffect } from 'react';
+
+import { DataSourcePluginOptionsEditorProps } from '@grafana/data';
+import { logDebug } from '@grafana/runtime';
+
+import { SQLConnectionDefaults } from '../../constants';
+import { SQLOptions } from '../../types';
+
+/**
+ * 1. Moves the database field from the options object to jsonData.database and empties the database field.
+ * 2. If max open connections, max idle connections, and auto idle are all undefined set these to default values.
+ */
+export function useMigrateDatabaseFields<T extends SQLOptions, S = {}>({
+  onOptionsChange,
+  options,
+}: DataSourcePluginOptionsEditorProps<T, S>) {
+  useEffect(() => {
+    const jsonData = options.jsonData;
+    let newOptions = { ...options };
+    let optionsUpdated = false;
+
+    // Migrate the database field from the column into the jsonData object
+    if (options.database) {
+      logDebug(`Migrating from options.database with value ${options.database} for ${options.name}`);
+      newOptions.database = '';
+      newOptions.jsonData = { ...jsonData, database: options.database };
+      optionsUpdated = true;
+    }
+
+    // Set default values for max open connections, max idle connection,
+    // and auto idle if they're all undefined
+    if (
+      jsonData.maxOpenConns === undefined &&
+      jsonData.maxIdleConns === undefined &&
+      jsonData.maxIdleConnsAuto === undefined
+    ) {
+      // It's expected that the default will be greater than 4
+      const maxOpenConns = SQLConnectionDefaults.MAX_CONNS;
+      const maxIdleConns = maxOpenConns;
+
+      logDebug(
+        `Setting default max open connections to ${maxOpenConns} and setting max idle connection to ${maxIdleConns}`
+      );
+
+      // Spread from the jsonData in new options in case
+      // the database field was migrated as well
+      newOptions.jsonData = {
+        ...newOptions.jsonData,
+        maxOpenConns: maxOpenConns,
+        maxIdleConns: maxIdleConns,
+        maxIdleConnsAuto: true,
+      };
+
+      // Make sure we issue an update if options changed
+      optionsUpdated = true;
+    }
+
+    // Only issue an update if we changed options
+    if (optionsUpdated) {
+      onOptionsChange(newOptions);
+    }
+  }, [onOptionsChange, options]);
+}

public/app/features/plugins/sql/constants.ts

@@ -15,3 +15,11 @@ export const MACRO_NAMES = [
   '$__unixEpochGroup',
   '$__unixEpochGroupAlias',
 ];
+
+/**
+ * Constants for SQL connection
+ * parameters and automatic settings
+ */
+export const SQLConnectionDefaults = {
+  MAX_CONNS: 100,
+};

public/app/features/plugins/sql/types.ts

@@ -30,6 +30,7 @@ export interface SqlQueryForInterpolation {
 export interface SQLConnectionLimits {
   maxOpenConns: number;
   maxIdleConns: number;
+  maxIdleConnsAuto: boolean;
   connMaxLifetime: number;
 }