Monitoring: Fix graph refresh on alert and rule details pages

Pass `query` and `ruleDuration` to `Graph` instead of the whole `rule` object to prevent some unnecessary re-rendering. This also removes the need to memoize `queries`. Memoize `labels` in `AlertsDetailsPage` to avoid triggering unnecessary re-renders of `Graph`. Initialize `pollInterval` to `undefined` instead of `null`. `null` indicates that polling was disabled, whereas `undefined` indicates that it is not specified and the `QueryBrowser` should use its default poll interval.
openshift · Oct 9, 2020 · b28adf9 · b28adf9
1 parent 96c2242
commit b28adf9
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 19 deletions.
diff --git a/frontend/public/components/monitoring/alerting.tsx b/frontend/public/components/monitoring/alerting.tsx
@@ -380,12 +380,11 @@ const queryBrowserURL = (query: string, namespace: string) =>
 const Graph_: React.FC<GraphProps> = ({
   deleteAll,
   filterLabels = undefined,
-  patchQuery,
-  rule,
   namespace,
+  patchQuery,
+  query,
+  ruleDuration,
 }) => {
-  const { duration = 0, query = '' } = rule || {};
-
   // Set the query in Redux so that other components like the graph tooltip can access it
   React.useEffect(() => {
     patchQuery(0, { query });
@@ -394,10 +393,8 @@ const Graph_: React.FC<GraphProps> = ({
   // Clear queries on unmount
   React.useEffect(() => deleteAll, [deleteAll]);
 
-  const queries = React.useMemo(() => [query], [query]);
-
   // 3 times the rule's duration, but not less than 30 minutes
-  const timespan = Math.max(3 * duration, 30 * 60) * 1000;
+  const timespan = Math.max(3 * ruleDuration, 30 * 60) * 1000;
 
   const GraphLink = () =>
     query ? <Link to={queryBrowserURL(query, namespace)}>View in Metrics</Link> : null;
@@ -407,7 +404,7 @@ const Graph_: React.FC<GraphProps> = ({
       defaultTimespan={timespan}
       filterLabels={filterLabels}
       GraphLink={GraphLink}
-      queries={queries}
+      queries={[query]}
     />
   );
 };
@@ -596,10 +593,15 @@ const alertStateToProps = (state: RootState, { match }): AlertsDetailsPageProps
 export const AlertsDetailsPage = withFallback(
   connect(alertStateToProps)((props: AlertsDetailsPageProps) => {
     const { alert, loaded, loadError, namespace, rule, silencesLoaded } = props;
-    const { annotations = {}, labels = {}, silencedBy = [] } = alert || {};
-    const { alertname, severity } = labels as any;
+    const annotations = alert?.annotations ?? {};
     const state = alertState(alert);
 
+    const alertLabels: PrometheusLabels = alert?.labels ?? {};
+    const labelsMemoKey = JSON.stringify(alertLabels);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    const labels: PrometheusLabels = React.useMemo(() => alertLabels, [labelsMemoKey]);
+    const { alertname, severity } = labels;
+
     return (
       <>
         <Helmet>
@@ -639,7 +641,12 @@ export const AlertsDetailsPage = withFallback(
             <div className="co-m-pane__body-group">
               <div className="row">
                 <div className="col-sm-12">
-                  <Graph filterLabels={labels} namespace={namespace} rule={rule} />
+                  <Graph
+                    filterLabels={labels}
+                    namespace={namespace}
+                    query={rule?.query}
+                    ruleDuration={rule?.duration}
+                  />
                 </div>
               </div>
               <div className="row">
@@ -733,15 +740,15 @@ export const AlertsDetailsPage = withFallback(
               </div>
             </div>
           </div>
-          {silencesLoaded && !_.isEmpty(silencedBy) && (
+          {silencesLoaded && !_.isEmpty(alert?.silencedBy) && (
             <div className="co-m-pane__body">
               <div className="co-m-pane__body-group">
                 <SectionHeading text="Silenced By" />
                 <div className="row">
                   <div className="col-xs-12">
                     <Table
                       aria-label="Silenced By"
-                      data={silencedBy}
+                      data={alert?.silencedBy}
                       Header={silenceTableHeaderNoSort}
                       loaded={true}
                       Row={SilenceTableRow}
@@ -910,15 +917,15 @@ export const AlertRulesDetailsPage = withFallback(
               <SectionHeading text="Active Alerts" />
               <div className="row">
                 <div className="col-sm-12">
-                  <Graph namespace={namespace} rule={rule} />
+                  <Graph namespace={namespace} query={rule?.query} ruleDuration={rule?.duration} />
                 </div>
               </div>
               <div className="row">
                 <div className="col-xs-12">
                   {_.isEmpty(alerts) ? (
                     <div className="text-center">None Found</div>
                   ) : (
-                    <ActiveAlerts alerts={alerts} ruleID={rule.id} namespace={namespace} />
+                    <ActiveAlerts alerts={alerts} ruleID={rule?.id} namespace={namespace} />
                   )}
                 </div>
               </div>
@@ -1645,7 +1652,8 @@ type GraphProps = {
   filterLabels?: PrometheusLabels;
   namespace?: string;
   patchQuery: (index: number, patch: QueryObj) => any;
-  rule: Rule;
+  query: string;
+  ruleDuration: number;
 };
 
 type MonitoringResourceIconProps = {

diff --git a/frontend/public/components/monitoring/poll-interval-dropdown.tsx b/frontend/public/components/monitoring/poll-interval-dropdown.tsx
@@ -32,7 +32,9 @@ const IntervalDropdown: React.FC<Props> = ({ interval, setInterval }) => {
     <Dropdown
       items={intervalOptions}
       onChange={onChange}
-      selectedKey={interval === null ? OFF_KEY : formatPrometheusDuration(interval)}
+      selectedKey={
+        interval === undefined || interval === null ? OFF_KEY : formatPrometheusDuration(interval)
+      }
     />
   );
 };

diff --git a/frontend/public/components/monitoring/query-browser.tsx b/frontend/public/components/monitoring/query-browser.tsx
@@ -552,7 +552,7 @@ const QueryBrowser_: React.FC<QueryBrowserProps> = ({
 
   const [containerRef, width] = useRefWidth();
 
-  const endTime = _.get(xDomain, '[1]');
+  const endTime = xDomain?.[1];
 
   const safeFetch = useSafeFetch();
 

diff --git a/frontend/public/reducers/ui.ts b/frontend/public/reducers/ui.ts
@@ -120,7 +120,7 @@ export default (state: UIState, action: UIAction): UIState => {
       }),
       queryBrowser: ImmutableMap({
         metrics: [],
-        pollInterval: null,
+        pollInterval: undefined,
         queries: ImmutableList([newQueryBrowserQuery()]),
       }),
       columnManagement: ImmutableMap(storedTableColumns),