feat: adhoc comparison UI (#580)

* refactor: clean-up adhoc single component. - Remove the unneeded header. - Move file upload callback to its own util to be reused. - Embed the file uploader in the flamebeearer. - Remove unused stuff. * feat: add an adhoc comparison component, based on comparison one. * Put the uploaded flamebearers in redux state * chore: style / typing fixes provided by the linter * chore: fix format * Fix typo Co-authored-by: eduardo aleixo <[email protected]> * Remove no longer needed comment Co-authored-by: eduardo aleixo <[email protected]> * Remove abortTimelineRequest from adhoc components. * Remove debug output Co-authored-by: eduardo aleixo <[email protected]> * Use the proper feature flag instead of the hardcoded one. * Set EnableExperimentalAdhocUI's default to true for the PR. * Add some basic sanity checks on uploaded file * Fix flamebearer upload checks. * Fix state handling in adhoc components. * Set EnableExperimentalAdhocUI to true in the test suite. * Add support for the adhoc route paths and sidebar links. adhoc-comparion-diff is commented for now, as it's yet unsupported. * Fix typo in interpolation. * Send a notification in case of parsing error. * Fix the notification and remove the alert. Thanks to @eh-am for spotting the problem. * Use flamebearer in file uploader component too. * Revert back EnableExperimentalAdhocUI to false by default. * Move FileUploader out of the FlameGraphRenderer component. Co-authored-by: eduardo aleixo <[email protected]>
grafana · Dec 15, 2021 · 3272249 · 3272249
1 parent 9a6f8da
commit 3272249
Show file tree

Hide file tree

Showing 11 changed files with 221 additions and 89 deletions.
diff --git a/pkg/cli/flags_test.go b/pkg/cli/flags_test.go
@@ -194,22 +194,23 @@ var _ = Describe("flags", func() {
 				err := exampleCommand.Execute()
 				Expect(err).ToNot(HaveOccurred())
 				Expect(cfg).To(Equal(config.Server{
-					AnalyticsOptOut:         false,
-					Config:                  "testdata/server.yml",
-					LogLevel:                "debug",
-					BadgerLogLevel:          "error",
-					StoragePath:             "/var/lib/pyroscope",
-					APIBindAddr:             ":4040",
-					BaseURL:                 "",
-					CacheEvictThreshold:     0.25,
-					CacheEvictVolume:        0.33,
-					BadgerNoTruncate:        false,
-					DisablePprofEndpoint:    false,
-					EnableExperimentalAdmin: true,
-					MaxNodesSerialization:   2048,
-					MaxNodesRender:          8192,
-					HideApplications:        []string{},
-					Retention:               0,
+					AnalyticsOptOut:           false,
+					Config:                    "testdata/server.yml",
+					LogLevel:                  "debug",
+					BadgerLogLevel:            "error",
+					StoragePath:               "/var/lib/pyroscope",
+					APIBindAddr:               ":4040",
+					BaseURL:                   "",
+					CacheEvictThreshold:       0.25,
+					CacheEvictVolume:          0.33,
+					BadgerNoTruncate:          false,
+					DisablePprofEndpoint:      false,
+					EnableExperimentalAdmin:   true,
+					EnableExperimentalAdhocUI: false,
+					MaxNodesSerialization:     2048,
+					MaxNodesRender:            8192,
+					HideApplications:          []string{},
+					Retention:                 0,
 					RetentionLevels: config.RetentionLevels{
 						Zero: 100 * time.Second,
 						One:  1000 * time.Second,

diff --git a/pkg/server/handler.go b/pkg/server/handler.go
@@ -203,6 +203,15 @@ func (ctrl *Controller) indexHandler() http.HandlerFunc {
 		} else if path == "/comparison-diff" {
 			ctrl.statsInc("diff")
 			ctrl.renderIndexPage(rw, r)
+		} else if path == "/adhoc-single" {
+			ctrl.statsInc("adhoc-index")
+			ctrl.renderIndexPage(rw, r)
+		} else if path == "/adhoc-comparison" {
+			ctrl.statsInc("adhoc-comparison")
+			ctrl.renderIndexPage(rw, r)
+		} else if path == "/adhoc-comparison-diff" {
+			ctrl.statsInc("adhoc-comparison-diff")
+			ctrl.renderIndexPage(rw, r)
 		} else {
 			fs.ServeHTTP(rw, r)
 		}

diff --git a/webapp/javascript/components/AdhocComparison.tsx b/webapp/javascript/components/AdhocComparison.tsx
@@ -0,0 +1,63 @@
+import React, { useEffect } from 'react';
+import { connect } from 'react-redux';
+import 'react-dom';
+
+import { bindActionCreators } from 'redux';
+import Box from '@ui/Box';
+import FileUploader from './FileUploader';
+import FlameGraphRenderer from './FlameGraph';
+import Footer from './Footer';
+import { setLeftFile, setRightFile } from '../redux/actions';
+import styles from './ComparisonApp.module.css';
+
+function AdhocComparison(props) {
+  const { actions, leftFile, leftFlamebearer, rightFile, rightFlamebearer } =
+    props;
+  const { setLeftFile } = actions;
+  const { setRightFile } = actions;
+
+  return (
+    <div className="pyroscope-app">
+      <div className="main-wrapper">
+        <div
+          className="comparison-container"
+          data-testid="comparison-container"
+        >
+          <Box className={styles.comparisonPane}>
+            <FileUploader file={leftFile} setFile={setLeftFile} />
+            <FlameGraphRenderer
+              viewType="double"
+              viewSide="left"
+              flamebearer={leftFlamebearer}
+              data-testid="flamegraph-renderer-left"
+            />
+          </Box>
+          <Box className={styles.comparisonPane}>
+            <FileUploader file={rightFile} setFile={setRightFile} />
+            <FlameGraphRenderer
+              viewType="double"
+              viewSide="right"
+              flamebearer={rightFlamebearer}
+              data-testid="flamegraph-renderer-right"
+            />
+          </Box>
+        </div>
+      </div>
+      <Footer />
+    </div>
+  );
+}
+
+const mapStateToProps = (state) => ({
+  ...state.root,
+  leftFile: state.root.adhocComparison.left.file,
+  leftFlamebearer: state.root.adhocComparison.left.flamebearer,
+  rightFile: state.root.adhocComparison.right.file,
+  rightFlamebearer: state.root.adhocComparison.right.flamebearer,
+});
+
+const mapDispatchToProps = (dispatch) => ({
+  actions: bindActionCreators({ setLeftFile, setRightFile }, dispatch),
+});
+
+export default connect(mapStateToProps, mapDispatchToProps)(AdhocComparison);
diff --git a/webapp/javascript/components/AdhocSingle.jsx b/webapp/javascript/components/AdhocSingle.jsx
@@ -1,57 +1,29 @@
-import React, { useEffect, useRef, useState } from 'react';
+import React, { useEffect } from 'react';
 import { connect } from 'react-redux';
-import Dropzone from 'react-dropzone';
 import 'react-dom';
 
 import { bindActionCreators } from 'redux';
+import Box from '@ui/Box';
+import FileUploader from './FileUploader';
 import FlameGraphRenderer from './FlameGraph';
-import Header from './Header';
 import Footer from './Footer';
-import { buildRenderURL } from '../util/updateRequests';
-import {
-  fetchNames,
-  fetchPyrescopeAppData,
-  abortTimelineRequest,
-} from '../redux/actions';
-import FileUploader from './FileUploader';
-import { deltaDiffWrapper } from '../util/flamebearer';
+import { setFile } from '../redux/actions';
 
 function AdhocSingle(props) {
-  const { actions, renderURL, single } = props;
-  const prevPropsRef = useRef();
-  const [flamebearer, setFlamebearer] = useState();
-
-  useEffect(() => {
-    if (prevPropsRef.renderURL !== renderURL) {
-      actions.fetchPyrescopeAppData(renderURL);
-    }
-
-    return actions.abortTimelineRequest;
-  }, [renderURL]);
-
-  const onFileUpload = (data) => {
-    if (!data) {
-      setFlamebearer(null);
-      return;
-    }
-
-    const { flamebearer } = data;
-
-    const calculatedLevels = deltaDiffWrapper(
-      flamebearer.format,
-      flamebearer.levels
-    );
-
-    flamebearer.levels = calculatedLevels;
-    setFlamebearer(flamebearer);
-  };
+  const { actions, file, flamebearer } = props;
+  const { setFile } = actions;
 
   return (
     <div className="pyroscope-app">
       <div className="main-wrapper">
-        <Header />
-        <FileUploader onUpload={onFileUpload} />
-        <FlameGraphRenderer flamebearer={flamebearer} viewType="single" />
+        <Box>
+          <FileUploader file={file} setFile={setFile} />
+          <FlameGraphRenderer
+            flamebearer={flamebearer}
+            uploader={{ file, setFile }}
+            viewType="single"
+          />
+        </Box>
       </div>
       <Footer />
     </div>
@@ -60,18 +32,12 @@ function AdhocSingle(props) {
 
 const mapStateToProps = (state) => ({
   ...state.root,
-  renderURL: buildRenderURL(state.root),
+  file: state.root.adhocSingle.file,
+  flamebearer: state.root.adhocSingle.flamebearer,
 });
 
 const mapDispatchToProps = (dispatch) => ({
-  actions: bindActionCreators(
-    {
-      fetchPyrescopeAppData,
-      fetchNames,
-      abortTimelineRequest,
-    },
-    dispatch
-  ),
+  actions: bindActionCreators({ setFile }, dispatch),
 });
 
 export default connect(mapStateToProps, mapDispatchToProps)(AdhocSingle);
diff --git a/webapp/javascript/components/FileUploader.tsx b/webapp/javascript/components/FileUploader.tsx
@@ -1,16 +1,20 @@
 /* eslint-disable react/jsx-props-no-spreading */
-import React, { useCallback, useState } from 'react';
+import React, { useCallback } from 'react';
+import { useDispatch } from 'react-redux';
 import { useDropzone } from 'react-dropzone';
-import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { faTrash } from '@fortawesome/free-solid-svg-icons/faTrash';
 import Button from '@ui/Button';
+import { addNotification } from '../redux/reducers/notifications';
 import styles from './FileUploader.module.scss';
+import { deltaDiffWrapper } from '../util/flamebearer';
 
 interface Props {
   onUpload: (s: string) => void;
+  file: File;
+  setFile: (file: File, flamebearer: Record<string, unknown>) => void;
 }
-export default function FileUploader({ onUpload }: Props) {
-  const [file, setFile] = useState();
+export default function FileUploader({ file, setFile }: Props) {
+  const dispatch = useDispatch();
 
   const onDrop = useCallback((acceptedFiles) => {
     if (acceptedFiles.length > 1) {
@@ -30,11 +34,26 @@ export default function FileUploader({ onUpload }: Props) {
           const s = JSON.parse(
             String.fromCharCode.apply(null, new Uint8Array(binaryStr))
           );
-          setFile({ file, ...s });
-          onUpload({ file, ...s });
+          // Only check for flamebearer fields, the rest of the file format is checked on decoding.
+          const fields = ['names', 'levels', 'numTicks', 'maxSelf'];
+          fields.forEach((field) => {
+            if (!(field in s.flamebearer))
+              throw new Error(
+                `Unable to parse uploaded file: field ${field} missing`
+              );
+          });
+          setFile(file, s);
         } catch (e) {
-          console.log(e);
-          alert(e);
+          dispatch(
+            addNotification({
+              message: e.message,
+              type: 'danger',
+              dismiss: {
+                duration: 0,
+                showIcon: true,
+              },
+            })
+          );
         }
       };
       reader.readAsArrayBuffer(file);
@@ -47,8 +66,7 @@ export default function FileUploader({ onUpload }: Props) {
   });
 
   const onRemove = () => {
-    setFile(null);
-    onUpload(null);
+    setFile(null, null);
   };
 
   return (
@@ -68,7 +86,7 @@ export default function FileUploader({ onUpload }: Props) {
       </div>
       {file && (
         <aside>
-          Currently analyzing file {file.file.path}
+          Currently analyzing file {file.path}
           &nbsp;
           <Button icon={faTrash} onClick={onRemove}>
             Remove

diff --git a/webapp/javascript/components/Sidebar.tsx b/webapp/javascript/components/Sidebar.tsx
@@ -68,7 +68,7 @@ export default function Sidebar2(props: SidebarProps) {
   const isAdhocActive =
     isRouteActive('/adhoc-single') ||
     isRouteActive('/adhoc-comparison') ||
-    isRouteActive('/adhoc-diff');
+    isRouteActive('/adhoc-comparison-diff');
 
   const adhoc = (
     <SubMenu
@@ -89,21 +89,25 @@ export default function Sidebar2(props: SidebarProps) {
         icon={<Icon icon={faWindowMaximize} />}
       >
         Single View
+        <NavLink to={{ pathname: '/adhoc-single', search }} exact />
       </MenuItem>
       <MenuItem
         data-testid="sidebar-adhoc-comparison"
         active={isRouteActive('/adhoc-comparison')}
         icon={<Icon icon={faColumns} />}
       >
         Comparison View
+        <NavLink to={{ pathname: '/adhoc-comparison', search }} exact />
       </MenuItem>
+      {/*
       <MenuItem
-        data-testid="sidebar-adhoc-diff"
-        active={isRouteActive('/adhoc-diff')}
+        data-testid="sidebar-adhoc-comparison-diff"
+        active={isRouteActive('/adhoc-comparison-diff')}
         icon={<Icon icon={faChartBar} />}
       >
         Diff View
       </MenuItem>
+       */}
     </SubMenu>
   );
 

diff --git a/webapp/javascript/index.jsx b/webapp/javascript/index.jsx
@@ -4,6 +4,7 @@ import React from 'react';
 import { Provider } from 'react-redux';
 import { Router, Switch, Route } from 'react-router-dom';
 import FPSStats from 'react-fps-stats';
+import { isExperimentalAdhocUIEnabled } from '@utils/features';
 import Notifications from '@ui/Notifications';
 import store from './redux/store';
 
@@ -12,6 +13,7 @@ import ComparisonApp from './components/ComparisonApp';
 import ComparisonDiffApp from './components/ComparisonDiffApp';
 import Sidebar from './components/Sidebar';
 import AdhocSingle from './components/AdhocSingle';
+import AdhocComparison from './components/AdhocComparison';
 import ServerNotifications from './components/ServerNotifications';
 
 import history from './util/history';
@@ -25,9 +27,6 @@ try {
   console.error(e);
 }
 
-// TODO fetch this from localstorage?
-const enableAdhoc = true;
-
 ReactDOM.render(
   <Provider store={store}>
     <Router history={history}>
@@ -45,11 +44,16 @@ ReactDOM.render(
           <Route path="/comparison-diff">
             <ComparisonDiffApp />
           </Route>
-          {enableAdhoc && (
+          {isExperimentalAdhocUIEnabled && (
             <Route path="/adhoc-single">
               <AdhocSingle />
             </Route>
           )}
+          {isExperimentalAdhocUIEnabled && (
+            <Route path="/adhoc-comparison">
+              <AdhocComparison />
+            </Route>
+          )}
         </Switch>
       </div>
     </Router>

diff --git a/webapp/javascript/redux/actionTypes.js b/webapp/javascript/redux/actionTypes.js
@@ -26,3 +26,6 @@ export const REQUEST_TAG_VALUES = 'REQUEST_TAG_VALUES';
 export const RECEIVE_TAG_VALUES = 'RECEIVE_TAG_VALUES';
 export const REQUEST_COMPARISON_TIMELINE = 'REQUEST_COMPARISON_TIMELINE';
 export const RECEIVE_COMPARISON_TIMELINE = 'RECEIVE_COMPARISON_TIMELINE';
+export const SET_FILE = 'SET_FILE';
+export const SET_LEFT_FILE = 'SET_LEFT_FILE';
+export const SET_RIGHT_FILE = 'SET_RIGHT_FILE';