ui, server: update cluster settings to include last update time

Resolves #76626 Previously it was difficult to determine from the cluster settings page which settings have been altered. This patch updates the cluster settings endpoint to include a last updated field, indicating when the setting was last altered, which allows the settings page to include that column in the table and is pre-sorted by this value. Release justification: Fairly minor changes for QoL upgrade Release note: None
cockroachdb · May 26, 2022 · 4a7565f · 4a7565f
1 parent e4ca082
commit 4a7565f
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 37 deletions.
diff --git a/docs/generated/http/full.md b/docs/generated/http/full.md
@@ -5723,6 +5723,7 @@ SettingsResponse is the response to SettingsRequest.
 | type | [string](#cockroach.server.serverpb.SettingsResponse-string) |  |  | [reserved](#support-status) |
 | description | [string](#cockroach.server.serverpb.SettingsResponse-string) |  |  | [reserved](#support-status) |
 | public | [bool](#cockroach.server.serverpb.SettingsResponse-bool) |  |  | [reserved](#support-status) |
+| last_updated | [google.protobuf.Timestamp](#cockroach.server.serverpb.SettingsResponse-google.protobuf.Timestamp) |  |  | [reserved](#support-status) |
 
 
 

diff --git a/pkg/server/admin.go b/pkg/server/admin.go
@@ -1750,21 +1750,50 @@ func (s *adminServer) Settings(
 		}
 	}
 
+	// Read the system.settings table to determine the settings for which we have
+	// explicitly set values -- the in-memory SV has the set and default values
+	// flattened for quick reads, but we'd only need the non-defaults for comparison.
+	alteredSettings := make(map[string]*time.Time)
+	if it, err := s.server.sqlServer.internalExecutor.QueryIteratorEx(
+		ctx, "read-setting", nil, /* txn */
+		sessiondata.InternalExecutorOverride{User: security.RootUserName()},
+		`SELECT name, "lastUpdated" FROM system.settings`,
+	); err != nil {
+		log.Warningf(ctx, "failed to read settings: %s", err)
+	} else {
+		var ok bool
+		for ok, err = it.Next(ctx); ok; ok, err = it.Next(ctx) {
+			row := it.Cur()
+			name := string(tree.MustBeDString(row[0]))
+			lastUpdated := row[1].(*tree.DTimestamp)
+			alteredSettings[name] = &lastUpdated.Time
+		}
+		if err != nil {
+			// No need to clear AlteredSettings map since we only make best
+			// effort to populate it.
+			log.Warningf(ctx, "failed to read settings: %s", err)
+		}
+	}
+
 	resp := serverpb.SettingsResponse{KeyValues: make(map[string]serverpb.SettingsResponse_Value)}
 	for _, k := range keys {
 		v, ok := settings.Lookup(k, lookupPurpose, settings.ForSystemTenant)
 		if !ok {
 			continue
 		}
+		var altered *time.Time
+		if val, ok := alteredSettings[k]; ok {
+			altered = val
+		}
 		resp.KeyValues[k] = serverpb.SettingsResponse_Value{
 			Type: v.Typ(),
 			// Note: v.String() redacts the values if the purpose is not "LocalAccess".
 			Value:       v.String(&s.server.st.SV),
 			Description: v.Description(),
 			Public:      v.Visibility() == settings.Public,
+			LastUpdated: altered,
 		}
 	}
-
 	return &resp, nil
 }
 

diff --git a/pkg/server/admin_test.go b/pkg/server/admin_test.go
@@ -1087,7 +1087,7 @@ func TestAdminAPISettings(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	s, _, _ := serverutils.StartServer(t, base.TestServerArgs{})
+	s, conn, _ := serverutils.StartServer(t, base.TestServerArgs{})
 	defer s.Stopper().Stop(context.Background())
 
 	// Any bool that defaults to true will work here.
@@ -1122,6 +1122,20 @@ func TestAdminAPISettings(t *testing.T) {
 		if typ != v.Type {
 			t.Errorf("%s: expected type %s, got %s", k, typ, v.Type)
 		}
+		if v.LastUpdated != nil {
+			db := sqlutils.MakeSQLRunner(conn)
+			q := makeSQLQuery()
+			q.Append(`SELECT name, "lastUpdated" FROM system.settings WHERE name=$`, k)
+			rows := db.Query(
+				t,
+				q.String(),
+				q.QueryArguments()...,
+			)
+			defer rows.Close()
+			if rows.Next() == false {
+				t.Errorf("missing sql row for %s", k)
+			}
+		}
 	}
 
 	t.Run("all", func(t *testing.T) {

diff --git a/pkg/server/serverpb/admin.proto b/pkg/server/serverpb/admin.proto
@@ -525,6 +525,7 @@ message SettingsResponse {
       string type = 2;
       string description = 3;
       bool public = 4;
+      google.protobuf.Timestamp last_updated = 5 [(gogoproto.nullable) = true, (gogoproto.stdtime) = true];
    }
    map<string, Value> key_values = 1 [(gogoproto.nullable) = false];
 }

diff --git a/pkg/ui/workspaces/db-console/src/views/reports/containers/settings/index.tsx b/pkg/ui/workspaces/db-console/src/views/reports/containers/settings/index.tsx
@@ -17,7 +17,14 @@ import { RouteComponentProps, withRouter } from "react-router-dom";
 import * as protos from "src/js/protos";
 import { refreshSettings } from "src/redux/apiReducers";
 import { AdminUIState } from "src/redux/state";
-import { Loading } from "@cockroachlabs/cluster-ui";
+import { DATE_FORMAT_24_UTC } from "src/util/format";
+import {
+  Loading,
+  ColumnDescriptor,
+  SortedTable,
+  SortSetting,
+  util,
+} from "@cockroachlabs/cluster-ui";
 import "./index.styl";
 import { CachedDataReducerState } from "src/redux/cachedDataReducer";
 
@@ -28,12 +35,37 @@ interface SettingsOwnProps {
   refreshSettings: typeof refreshSettings;
 }
 
+interface IterableSetting {
+  key: string;
+  description?: string;
+  type?: string;
+  value?: string;
+  public?: boolean;
+  last_updated?: moment.Moment;
+}
+
+interface SettingsState {
+  sortSetting: {
+    ascending: boolean;
+    columnTitle: string;
+  };
+}
+
 type SettingsProps = SettingsOwnProps & RouteComponentProps;
 
 /**
  * Renders the Cluster Settings Report page.
  */
-export class Settings extends React.Component<SettingsProps> {
+export class Settings extends React.Component<SettingsProps, SettingsState> {
+  constructor(props: SettingsProps) {
+    super(props);
+    this.state = {
+      sortSetting: { ascending: true, columnTitle: "lastUpdated" },
+    };
+  }
+
+  sortSetting: { ascending: boolean; columnTitle: string | null };
+
   refresh(props = this.props) {
     props.refreshSettings(
       new protos.cockroach.server.serverpb.SettingsRequest(),
@@ -51,41 +83,63 @@ export class Settings extends React.Component<SettingsProps> {
     }
 
     const { key_values } = this.props.settings.data;
-    const data: any = _.keys(key_values);
+    const dataArray: IterableSetting[] = Object.keys(key_values)
+      .map(key => ({
+        key,
+        ...key_values[key],
+      }))
+      .map(obj => {
+        return {
+          ...obj,
+          last_updated: obj.last_updated
+            ? util.TimestampToMoment(obj.last_updated)
+            : null,
+        };
+      });
+    const columns: ColumnDescriptor<IterableSetting>[] = [
+      {
+        name: "name",
+        title: "Setting",
+        cell: (setting: IterableSetting) => setting.key,
+        sort: (setting: IterableSetting) => setting.key,
+      },
+      {
+        name: "value",
+        title: "Value",
+        cell: (setting: IterableSetting) => setting.value,
+      },
+      {
+        name: "lastUpdated",
+        title: "Last Updated",
+        cell: (setting: IterableSetting) =>
+          setting.last_updated
+            ? setting.last_updated.format(DATE_FORMAT_24_UTC)
+            : "No overrides",
+        sort: (setting: IterableSetting) => setting.last_updated?.valueOf(),
+      },
+      {
+        name: "description",
+        title: "Description",
+        cell: (setting: IterableSetting) => setting.description,
+      },
+    ];
 
     return (
-      <table className="settings-table">
-        <thead>
-          <tr className="settings-table__row settings-table__row--header">
-            <th className="settings-table__cell settings-table__cell--header">
-              Setting
-            </th>
-            <th className="settings-table__cell settings-table__cell--header">
-              Value
-            </th>
-            <th className="settings-table__cell settings-table__cell--header">
-              Description
-            </th>
-          </tr>
-        </thead>
-        <tbody>
-          {_.chain(data)
-            .filter(key => key_values[key].public === wantPublic)
-            .sort()
-            .map((key: number) => (
-              <tr key={key} className="settings-table__row">
-                <td className="settings-table__cell">{key}</td>
-                <td className="settings-table__cell">
-                  {key_values[key].value}
-                </td>
-                <td className="settings-table__cell">
-                  {key_values[key].description}
-                </td>
-              </tr>
-            ))
-            .value()}
-        </tbody>
-      </table>
+      <SortedTable
+        data={dataArray.filter(obj =>
+          wantPublic ? obj.public : obj.public === undefined,
+        )}
+        columns={columns}
+        sortSetting={this.state.sortSetting}
+        onChangeSortSetting={(ss: SortSetting) =>
+          this.setState({
+            sortSetting: {
+              ascending: ss.ascending,
+              columnTitle: ss.columnTitle,
+            },
+          })
+        }
+      />
     );
   }