Config page design adjustments (#1422)

alyssajoyner · web-flow · commit 34b0c3044814 · 2025-11-10T15:02:51.000-07:00
diff --git a/README.md b/README.md
@@ -32,7 +32,7 @@ To configure a readonly user, follow these steps:
 
 1. Create a `readonly` user profile following the [Creating Users and Roles in ClickHouse](https://clickhouse.com/docs/en/operations/access-rights) guide.
 2. Ensure the `readonly` user has enough permission to modify the `max_execution_time` setting required by the underlying [clickhouse-go client](https://github.com/ClickHouse/clickhouse-go/).
-3. If you're using a public Clickhouse instance, it's not recommended to set `readonly=2` in the `readonly` profile. Instead, leave `readonly=1` and set the constraint type of `max_execution_time` to [changeable_in_readonly](https://clickhouse.com/docs/en/operations/settings/constraints-on-settings) to allow modification of this setting.
+3. If you're using a public ClickHouse instance, it's not recommended to set `readonly=2` in the `readonly` profile. Instead, leave `readonly=1` and set the constraint type of `max_execution_time` to [changeable_in_readonly](https://clickhouse.com/docs/en/operations/settings/constraints-on-settings) to allow modification of this setting.
 
 ### ClickHouse protocol support
 
@@ -260,7 +260,7 @@ docs](https://grafana.com/docs/grafana/latest/variables/variable-types/add-ad-ho
 
 #### Using a query for Ad Hoc filters
 
-The second `clickhouse_adhoc_query` also allows any valid Clickhouse query. The
+The second `clickhouse_adhoc_query` also allows any valid ClickHouse query. The
 query results will be used to populate your ad-hoc filter's selectable filters.
 You may choose to hide this variable from view as it serves no further purpose.
 
diff --git a/src/components/configEditor/LogsConfig.test.tsx b/src/components/configEditor/LogsConfig.test.tsx
@@ -3,7 +3,7 @@ import { render, fireEvent } from '@testing-library/react';
 import { LogsConfig } from './LogsConfig';
 import allLabels from 'labels';
 import { columnLabelToPlaceholder } from 'data/utils';
-import { defaultLogsTable } from 'otel';
+import { defaultCHAdditionalSettingsConfig } from 'types/config';
 
 describe('LogsConfig', () => {
   it('should render', () => {
@@ -68,7 +68,7 @@ describe('LogsConfig', () => {
     );
     expect(result.container.firstChild).not.toBeNull();
 
-    const input = result.getByPlaceholderText(defaultLogsTable);
+    const input = result.getByPlaceholderText(defaultCHAdditionalSettingsConfig.logs?.defaultTable!);
     expect(input).toBeInTheDocument();
     fireEvent.change(input, { target: { value: 'changed' } });
     fireEvent.blur(input);
diff --git a/src/components/configEditor/LogsConfig.tsx b/src/components/configEditor/LogsConfig.tsx
@@ -3,9 +3,9 @@ import { ConfigSection, ConfigSubSection } from 'components/experimental/ConfigS
 import { Input, Field, InlineFormLabel, TagsInput } from '@grafana/ui';
 import { OtelVersionSelect } from 'components/queryBuilder/OtelVersionSelect';
 import { ColumnHint } from 'types/queryBuilder';
-import otel, { defaultLogsTable } from 'otel';
+import otel from 'otel';
 import { LabeledInput } from './LabeledInput';
-import { CHLogsConfig } from 'types/config';
+import { CHLogsConfig, defaultCHAdditionalSettingsConfig } from 'types/config';
 import allLabels from 'labels';
 import { columnLabelToPlaceholder } from 'data/utils';
 import { Switch } from 'components/queryBuilder/Switch';
@@ -80,7 +80,7 @@ export const LogsConfig = (props: LogsConfigProps) => {
           onChange={(e) => onDefaultTableChange(e.currentTarget.value)}
           label={labels.defaultTable.label}
           aria-label={labels.defaultTable.label}
-          placeholder={defaultLogsTable}
+          placeholder={defaultCHAdditionalSettingsConfig.logs?.defaultTable!}
         />
       </Field>
       <ConfigSubSection title={labels.columns.title} description={labels.columns.description}>
diff --git a/src/components/configEditor/TracesConfig.test.tsx b/src/components/configEditor/TracesConfig.test.tsx
@@ -3,7 +3,7 @@ import { render, fireEvent } from '@testing-library/react';
 import { TracesConfig, TraceConfigProps } from './TracesConfig';
 import allLabels from 'labels';
 import { columnLabelToPlaceholder } from 'data/utils';
-import { defaultTraceTable } from 'otel';
+import { defaultCHAdditionalSettingsConfig } from 'types/config';
 
 function defaultTraceConfigProps(): TraceConfigProps {
   return {
@@ -60,7 +60,7 @@ describe('TracesConfig', () => {
     const result = render(<TracesConfig {...defaultTraceConfigProps()} onDefaultTableChange={onDefaultTableChange} />);
     expect(result.container.firstChild).not.toBeNull();
 
-    const input = result.getByPlaceholderText(defaultTraceTable);
+    const input = result.getByPlaceholderText(defaultCHAdditionalSettingsConfig.traces?.defaultTable!);
     expect(input).toBeInTheDocument();
     fireEvent.change(input, { target: { value: 'changed' } });
     fireEvent.blur(input);
diff --git a/src/components/configEditor/TracesConfig.tsx b/src/components/configEditor/TracesConfig.tsx
@@ -3,10 +3,10 @@ import { ConfigSection, ConfigSubSection } from 'components/experimental/ConfigS
 import { Input, Field } from '@grafana/ui';
 import { OtelVersionSelect } from 'components/queryBuilder/OtelVersionSelect';
 import { ColumnHint, TimeUnit } from 'types/queryBuilder';
-import otel, { defaultTraceTable } from 'otel';
+import otel from 'otel';
 import { LabeledInput } from './LabeledInput';
 import { DurationUnitSelect } from 'components/queryBuilder/DurationUnitSelect';
-import { CHTracesConfig } from 'types/config';
+import { CHTracesConfig, defaultCHAdditionalSettingsConfig } from 'types/config';
 import allLabels from 'labels';
 import { columnLabelToPlaceholder } from 'data/utils';
 import { Switch } from 'components/queryBuilder/Switch';
@@ -136,7 +136,7 @@ export const TracesConfig = (props: TraceConfigProps) => {
           onChange={(e) => onDefaultTableChange(e.currentTarget.value)}
           label={labels.defaultTable.label}
           aria-label={labels.defaultTable.label}
-          placeholder={defaultTraceTable}
+          placeholder={defaultCHAdditionalSettingsConfig.traces?.defaultTable!}
         />
       </Field>
       <ConfigSubSection title={labels.columns.title} description={labels.columns.description}>
@@ -197,7 +197,7 @@ export const TracesConfig = (props: TraceConfigProps) => {
         />
         <DurationUnitSelect
           disabled={otelEnabled}
-          unit={(durationUnit as TimeUnit) || TimeUnit.Nanoseconds}
+          unit={(durationUnit as TimeUnit) || defaultCHAdditionalSettingsConfig.traces?.durationUnit}
           onChange={onDurationUnitChange}
         />
         <LabeledInput
diff --git a/src/module.ts b/src/module.ts
@@ -10,7 +10,7 @@ import { analyzeQueries, trackClickhouseDashboardLoaded } from 'tracking';
 import pluginJson from './plugin.json';
 import clickhouseVersion from '../package.json';
 
-// ConfigEditorV2 is the new design for the Clickhouse configuration page
+// ConfigEditorV2 is the new design for the ClickHouse configuration page
 const configEditor = config.featureToggles['newClickhouseConfigPageDesign' as keyof FeatureToggles]
   ? ConfigEditorV2
   : ConfigEditorV1;
diff --git a/src/selectors.ts b/src/selectors.ts
@@ -47,11 +47,11 @@ export const Components = {
       },
       DATABASE: {
         label: 'Database',
-        tooltip: 'Clickhouse database to query from',
+        tooltip: 'ClickHouse database to query from',
       },
       FROM: {
         label: 'Table',
-        tooltip: 'Clickhouse table to query from',
+        tooltip: 'ClickHouse table to query from',
       },
       SELECT: {
         label: 'Fields',
diff --git a/src/types/config.ts b/src/types/config.ts
@@ -1,4 +1,6 @@
 import { DataSourceJsonData, KeyValue } from '@grafana/data';
+import otel, { defaultLogsTable, defaultTraceTable } from 'otel';
+import { TimeUnit } from './queryBuilder';
 
 export interface CHConfig extends DataSourceJsonData {
   /**
@@ -118,3 +120,17 @@ export enum Protocol {
   Native = 'native',
   Http = 'http',
 }
+
+export const defaultCHAdditionalSettingsConfig: Partial<CHConfig> = {
+  logs: {
+    defaultTable: defaultLogsTable,
+    otelVersion: otel.getLatestVersion().version,
+    selectContextColumns: true,
+    contextColumns: [],
+  },
+  traces: {
+    defaultTable: defaultTraceTable,
+    otelVersion: otel.getLatestVersion().version,
+    durationUnit: TimeUnit.Nanoseconds,
+  },
+};
diff --git a/src/views/CHConfigEditorHooks.test.ts b/src/views/CHConfigEditorHooks.test.ts
@@ -1,9 +1,8 @@
 import { DataSourceSettings } from '@grafana/data';
 import { renderHook } from '@testing-library/react';
-import { CHConfig, CHHttpHeader, CHSecureConfig, Protocol } from 'types/config';
+import { CHConfig, CHHttpHeader, CHSecureConfig, defaultCHAdditionalSettingsConfig, Protocol } from 'types/config';
 import { onHttpHeadersChange, useConfigDefaults } from './CHConfigEditorHooks';
 import { pluginVersion } from 'utils/version';
-import { defaultLogsTable, defaultTraceTable } from 'otel';
 
 describe('onHttpHeadersChange', () => {
   it('should properly sort headers into secure/plain config fields', async () => {
@@ -95,12 +94,12 @@ describe('useConfigDefaults', () => {
     version: pluginVersion,
     protocol: Protocol.Native,
     logs: {
-      defaultTable: defaultLogsTable,
+      defaultTable: defaultCHAdditionalSettingsConfig.logs?.defaultTable,
       selectContextColumns: true,
       contextColumns: [],
     },
     traces: {
-      defaultTable: defaultTraceTable,
+      defaultTable: defaultCHAdditionalSettingsConfig.traces?.defaultTable,
     },
   };
 
diff --git a/src/views/CHConfigEditorHooks.ts b/src/views/CHConfigEditorHooks.ts
@@ -1,7 +1,6 @@
 import { DataSourceSettings, KeyValue } from '@grafana/data';
-import { defaultLogsTable, defaultTraceTable } from 'otel';
 import { useEffect, useRef } from 'react';
-import { CHConfig, CHHttpHeader, CHSecureConfig, Protocol } from 'types/config';
+import { CHConfig, CHHttpHeader, CHSecureConfig, defaultCHAdditionalSettingsConfig, Protocol } from 'types/config';
 import { pluginVersion } from 'utils/version';
 
 /**
@@ -103,7 +102,7 @@ export const useConfigDefaults = (
     if (!jsonData.logs || jsonData.logs.defaultTable === undefined) {
       jsonData.logs = {
         ...jsonData.logs,
-        defaultTable: defaultLogsTable,
+        defaultTable: defaultCHAdditionalSettingsConfig.logs?.defaultTable,
         selectContextColumns: true,
         contextColumns: [],
       };
@@ -112,7 +111,7 @@ export const useConfigDefaults = (
     if (!jsonData.traces || jsonData.traces.defaultTable === undefined) {
       jsonData.traces = {
         ...jsonData.traces,
-        defaultTable: defaultTraceTable,
+        defaultTable: defaultCHAdditionalSettingsConfig.traces?.defaultTable,
       };
     }
 
diff --git a/src/views/config-v2/AdditionalSettingsSection.tsx b/src/views/config-v2/AdditionalSettingsSection.tsx
@@ -1,12 +1,20 @@
 import { ConfigSubSection } from 'components/experimental/ConfigSection';
 import allLabels from './labelsV2';
-import React, { ChangeEvent, useState } from 'react';
+import React, { ChangeEvent, useMemo, useState } from 'react';
 import {
   DataSourcePluginOptionsEditorProps,
   onUpdateDatasourceJsonDataOption,
   onUpdateDatasourceJsonDataOptionChecked,
 } from '@grafana/data';
-import { AliasTableEntry, CHConfig, CHCustomSetting, CHLogsConfig, CHSecureConfig, CHTracesConfig } from 'types/config';
+import {
+  AliasTableEntry,
+  CHConfig,
+  CHCustomSetting,
+  CHLogsConfig,
+  CHSecureConfig,
+  CHTracesConfig,
+  defaultCHAdditionalSettingsConfig,
+} from 'types/config';
 import { AliasTableConfig } from 'components/configEditor/AliasTableConfig';
 import { DefaultDatabaseTableConfig } from 'components/configEditor/DefaultDatabaseTableConfig';
 import { LogsConfig } from 'components/configEditor/LogsConfig';
@@ -110,6 +118,50 @@ export const AdditionalSettingsSection = (props: Props) => {
     });
   };
 
+  const shallowSettingsCompare = (currentSettings: any, defaultSettings: any): boolean => {
+    // needed for dealing with proxy object from currentSettings
+    currentSettings = Object.assign({}, currentSettings);
+
+    const currentSettingsKeys = Object.keys(currentSettings);
+    const defaultSettingsKeys = Object.keys(defaultSettings);
+
+    if (currentSettingsKeys.length !== defaultSettingsKeys.length) {
+      return false;
+    }
+
+    for (const key of currentSettingsKeys) {
+      if (!defaultSettingsKeys.includes(key)) {
+        return false;
+      }
+      if (currentSettings[key].length === 0 && defaultSettings[key].length === 0) {
+        continue;
+      }
+      if (currentSettings[key] !== defaultSettings[key]) {
+        return false;
+      }
+    }
+    return true;
+  };
+
+  const shouldBeOpen = useMemo(() => {
+    return (
+      jsonData.defaultDatabase ||
+      jsonData.defaultTable ||
+      jsonData.connMaxLifetime ||
+      jsonData.dialTimeout ||
+      jsonData.maxIdleConns ||
+      jsonData.maxOpenConns ||
+      jsonData.queryTimeout ||
+      jsonData.validateSql ||
+      !shallowSettingsCompare(jsonData.logs, defaultCHAdditionalSettingsConfig.logs) ||
+      !shallowSettingsCompare(jsonData.traces, defaultCHAdditionalSettingsConfig.traces) ||
+      (jsonData.aliasTables && jsonData.aliasTables.length > 0) ||
+      jsonData.enableRowLimit ||
+      jsonData.enableSecureSocksProxy ||
+      customSettings.length > 0
+    );
+  }, [jsonData, customSettings]);
+
   return (
     <Box
       borderStyle="solid"
@@ -122,11 +174,11 @@ export const AdditionalSettingsSection = (props: Props) => {
       <CollapsableSection
         label={
           <>
-            <Text variant="h3">4. {CONFIG_SECTION_HEADERS[3].label}</Text>
+            <Text variant="h3">{CONFIG_SECTION_HEADERS[3].label}</Text>
             <Badge text="optional" color="darkgrey" className={styles.badge} />
           </>
         }
-        isOpen={!!CONFIG_SECTION_HEADERS[3].isOpen}
+        isOpen={!!shouldBeOpen}
       >
         <DefaultDatabaseTableConfig
           defaultDatabase={jsonData.defaultDatabase}
@@ -233,7 +285,7 @@ export const AdditionalSettingsSection = (props: Props) => {
           <Field label={labels.secureSocksProxy.label} description={labels.secureSocksProxy.tooltip}>
             <Switch
               value={jsonData.enableSecureSocksProxy || false}
-              onChange={(e) => onUpdateDatasourceJsonDataOption(props, 'enableSecureSocksProxy')(e)}
+              onChange={(e) => onUpdateDatasourceJsonDataOptionChecked(props, 'enableSecureSocksProxy')(e)}
             />
           </Field>
         )}
diff --git a/src/views/config-v2/CHConfigEditor.tsx b/src/views/config-v2/CHConfigEditor.tsx
@@ -28,7 +28,7 @@ export const ConfigEditor: React.FC<ConfigEditorProps> = (props) => {
         <div className={styles.requiredFields}>
           <Alert
             severity="info"
-            title="You are viewing a new design for the Clickhouse configuration settings."
+            title="You are viewing a new design for the ClickHouse configuration settings."
             className={styles.alertHeight}
           >
             <>
diff --git a/src/views/config-v2/DatabaseCredentialsSection.tsx b/src/views/config-v2/DatabaseCredentialsSection.tsx
@@ -47,7 +47,7 @@ export const DatabaseCredentialsSection = (props: Props) => {
       minWidth={CONTAINER_MIN_WIDTH}
     >
       <CollapsableSection
-        label={<Text variant="h3">2. {CONFIG_SECTION_HEADERS[1].label}</Text>}
+        label={<Text variant="h3">{CONFIG_SECTION_HEADERS[1].label}</Text>}
         isOpen={!!CONFIG_SECTION_HEADERS[1].isOpen}
       >
         <div className={styles.credentialsSection}>
@@ -77,7 +77,10 @@ export const DatabaseCredentialsSection = (props: Props) => {
               onBlur={trackClickhouseConfigV2DatabaseCredentialsUserInput}
             />
           </Field>
-          <Field label={labels.password.label} description={labels.password.tooltip}>
+          <Field
+            label={labels.password.label}
+            description={<div className={styles.passwordDescription}>{labels.password.tooltip}</div>}
+          >
             <SecretInput
               name="pwd"
               label={labels.password.label}
@@ -97,6 +100,9 @@ export const DatabaseCredentialsSection = (props: Props) => {
 };
 
 const getStyles = () => ({
+  passwordDescription: css({
+    marginTop: '5px',
+  }),
   credentialsSection: css({
     display: 'flex',
     flexWrap: 'wrap',
diff --git a/src/views/config-v2/LeftSidebar.tsx b/src/views/config-v2/LeftSidebar.tsx
diff --git a/src/views/config-v2/ServerAndEncryptionSection.tsx b/src/views/config-v2/ServerAndEncryptionSection.tsx
diff --git a/src/views/config-v2/TLSSSLSettingsSection.tsx b/src/views/config-v2/TLSSSLSettingsSection.tsx
diff --git a/src/views/config-v2/constants.ts b/src/views/config-v2/constants.ts
diff --git a/src/views/config-v2/helpers.ts b/src/views/config-v2/helpers.ts
diff --git a/src/views/config-v2/labelsV2.ts b/src/views/config-v2/labelsV2.ts
