Adding heatmap and graph_type #134

jlampar · 2018-05-08T14:19:36Z

Added new visualisation - heatmap (Grid). Also the @gomoripeti backend_graph_type branch is integrated in that PR (changes pasted inside appropriate files). The Grid component was very large in size so in the end it is broken into two pieces: a graph itself and helper functions (both are inside Grid folder).
This solves #107.

jlampar · 2018-05-09T06:57:40Z

I have to fix something. I've just realised that unfortunately all grid graphs are awkwardly landing in one container.

jlampar · 2018-05-09T08:47:17Z

Graphs landing in one container problem was solved by more uniquely ID distribution (as the D3 is working on the document level, not on the component level, it went crazy seeing multiple, same ID's).
Post Scriptum: I consider this version as a temporary one although fully functional. I can try to exchange the cycle of redrawing the graph to cycle of recolouring it (probably a lot less resource-consuming). Our previous C3 graph was in fact redrawing itself - I tried to create something in a similar fashion. I can try archiving this now - in this PR - or some time later. What do you think?

gomoripeti

great job

gomoripeti · 2018-05-09T20:39:27Z

apps/xprof_gui/priv/src/components/monitoring/Graph/Graph.jsx

+import Grid from './Grid/Grid';
+
+function composeID(q) {
+  const modFuncArity = q.split(':');


you shouldn't assume anything about the m.query it can have different format depending on Erlang or Elixir mode (eg Mod.fun/arity in case of Elixir) and it can have more advanced formats than just mod-fun-arity (eg #funcount mfa = mod:fun(1, _) )
I would be better to use m.mfa to compose the ID

there used to be a utility function chartId which we stopped using, but it can be reintroduced for safe IDs https://github.com/Appliscale/xprof/blob/master/priv/app/utils.js#L2

I'll use the chartId(MFA) function. If it worked before, it will work fine. Nevertheless I'll reintegrate it and test.

gomoripeti · 2018-05-09T20:39:31Z

apps/xprof_gui/priv/src/components/monitoring/Graph/Graph.jsx


 const propTypes = {
  dps: PropTypes.arrayOf(PropTypes.object).isRequired,
+  type: PropTypes.string.isRequired,
+  query: PropTypes.string.isRequired,


Maybe this can be the whole m json object (including mfa and query fields) to address above issue with composing the ID
(we need to find a good name for the json object, I dont know if function is good enough, in generic sense this is a monitor or monitored-object, which can be an m:f/a but also can be garbage_collection or memory in the future)

If we are composing the ID inside GraphPanel, in the end we are only passing the ID to the Graph and the panel has access to the whole m object. The panel needs the object as it decomposes it into the grid props, whereas the grid just receives stuff from the panel.
In case of renaming the JSON object I think this is a good idea, as for now the object named mfa has also a mfa property which is a little bit confusing (that's why I renamed it somewhere to m in loops or arguments).
I propose we:

rename the mfas to monitoredCollection,

rename the mfa to monitored.

What do you think?

right, so query is only there so that the Graph can show it as a title. +1 for renamings

amamla · 2018-05-10T08:52:40Z

apps/xprof_gui/priv/src/actions/CollectingActions.js

@@ -29,11 +29,13 @@ export const getMonitoredFunctions = () => async (dispatch, getState) => {
  if (error) {
    console.log('ERROR: ', error);
  } else if (!isEqual(mfas, json)) {
-    const newMfas = json.filter(mfa => !mfas.map(f => f[3]).includes(mfa[3]));
+    const newMfas = json
+      .filter(mfa => !mfas.map(f => f.query)


nitpicking: you could perform and collect mfas.map(f => f.query) before filtering to do it only once instead of doing it for each mfa from incomming json

Now it is:

const queries = mfas.map(f => f.query); const newMfas = json .filter(mfa => !queries .includes(mfa.query));

(for the sake of clarity, given in old, already changed naming convention)

MarcinKasprowicz · 2018-05-21T13:05:06Z

apps/xprof_gui/priv/src/components/monitoring/Graph/Graph.jsx

+  const queryID = composeID(query);
+  const wrapperID = `graphWrapper-${queryID}`;
+  const wrapper = document.getElementById(wrapperID);
+  if (type === 'grid') {


We can create constant for types and put it somewhere in src/constants

export const GRAPH_TYPES = { GRID: 'grid', LINE: 'line', }

MarcinKasprowicz · 2018-05-21T13:12:30Z

apps/xprof_gui/priv/src/components/monitoring/Graph/Graph.jsx

+          outerWidth={wrapper.clientWidth}
+          graphID={passId}
+        />}
+      </div>


As we introduced new type of graph we can add new layer of components here

switch (type) { case GRAPH_TYPES.GRID: return (<GridGraph />); case GRAPH_TYPES.LINE: return (<LineGraph />); default: return }

MarcinKasprowicz · 2018-05-21T13:27:16Z

apps/xprof_gui/priv/src/components/monitoring/Graph/Grid/Grid.jsx

@@ -0,0 +1,416 @@
+import React from 'react';


To keep convention please move Grid/ folder to monitoring/ (one level higher):
apps/xprof_gui/priv/src/components/monitoring/Grid/Grid.jsx

src components components in container1 component1 component2 ... containers container1 container2 ...

MarcinKasprowicz · 2018-05-21T13:33:45Z

apps/xprof_gui/priv/src/components/monitoring/Graph/Grid/Grid.jsx

+class Grid extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {


For keeping application's state we are using Redux, not internal compoment's state. We should treat component as pure functions

MarcinKasprowicz · 2018-05-21T13:35:52Z

apps/xprof_gui/priv/src/components/monitoring/Monitoring/Monitoring.jsx

@@ -26,10 +38,31 @@ const propTypes = {
 };

 class Monitoring extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {


For keeping application's state we are using Redux, not internal compoment's state

MarcinKasprowicz · 2018-05-21T13:44:30Z

apps/xprof_gui/priv/src/components/monitoring/Graph/Grid/Grid.jsx

+  graphID: PropTypes.number.isRequired,
+};
+
+export default Grid;


A lot is happening in this component, we should keep component's code short and readable. My proposal is to move all state-related logic into actions and whole d3-related logic to src/utils

I have to give a brief update considering this. Currently I'm working on making the component more flexible and thus I've also cleared this mess, adding three, D3-heavy methods to the class: compose(props), update() and reCompose(props) which are evoked from the lifecycle methods. Without much explanation it looks like this:

componentWillMount() { window.addEventListener('resize', this.updateWindowDimensions); this.updateWindowDimensions(); prepareTooltip(); } componentDidMount() { this.compose(this.props); } componentWillReceiveProps(nextProps) { if (nextProps !== this.props) { this.updateWindowDimensions(); this.reCompose(nextProps); } } componentWillUpdate() { this.update(); } componentWillUnmount() { window.removeEventListener('resize', this.updateWindowDimensions); } [and then go all the methods... ]

What is the best way? Should I keep those composing functions as methods of the Grid component or split it between actions and src/utils?

Or maybe the better way is to move all D3-logic to the gridUtils and have only one place where the D3 is imported?

MarcinKasprowicz · 2018-05-21T13:49:09Z

apps/xprof_gui/priv/src/components/monitoring/Monitoring/Monitoring.jsx

@@ -3,6 +3,14 @@ import PropTypes from 'prop-types';
 import { GraphPanel } from '../';
 import { DATA_INTERVAL } from '../../../constants';

+function roll(func) {


Move it to utils

… addHeatmap

jlampar · 2018-06-05T09:32:22Z

GRAPH_TYPE constants were added and placed in the constants folder.
New layer of components was added with switch taking the GRAPH_TYPE constants into consideration - resolving whether it is line graph or grid graph.
The Grid folder was moved up. The new folders for the graph components were placed accordingly.
Components state is now managed by Redux and passed through props. No internal components state.
The gargantuan Grid component was reduced in complexity. The D3 logic resides now in two files:

gridComposingUtils.js where all the compose and recompose code is stored (the majority of D3 SVG creation and recreation flow)
gridUtils.js where all the small helper functions are.

The tests were a little bit fixed to fit the new form of Erlang output (object instead of an array).

jlampar · 2018-06-08T10:17:42Z

Ok, so all tests are passed. Additionally for the sake of explanation, the omnipresent GRAPH_INITIAL_SIZE constant was added because of strange logs I've got all the way. All the time it was logging that the size prop was undefined although it is required (but it came from the Redux state in which it has a defined initial state - it somehow ignored it). And the size prop is constitutional for the grid to be created.

MarcinKasprowicz · 2018-06-11T07:55:12Z

Great work!!

MarcinKasprowicz · 2018-06-11T07:29:06Z

apps/xprof_gui/priv/src/components/monitoring/Graph/Graph.jsx

+import GridGraph from '../GridGraph';
+import LineGraph from '../LineGraph';
+import { GRAPH_TYPE } from '../../../constants/GraphTypes';
+import { GRAPH_INITIAL_SIZE } from '../../../constants';


import { GRAPH_INITIAL_SIZE, GRAPH_TYPE } from '../../../constants';

MarcinKasprowicz · 2018-06-11T07:39:24Z

apps/xprof_gui/priv/src/components/monitoring/GraphPanel/GraphPanel.jsx

+  setSize,
+  size,
+}) => {
+  const monitoredID = safecomposeID(monitored.query);


Move monitoredID to the state. Dispatch an action in getMonitoredFunctions which will create/delete IDs.

adding heatmap and graph_type

1bc5860

jlampar requested review from gomoripeti and MarcinKasprowicz May 8, 2018 14:19

fixed one-container bug

e326cc4

minor Graph refactoring

2ba0fed

gomoripeti requested changes May 9, 2018

View reviewed changes

amamla reviewed May 10, 2018

View reviewed changes

jlampar added 3 commits May 10, 2018 13:26

integrated chartID, renaming mfa/mfas, filtering

663693a

fixing async issue

0f54638

changed re-creation into updating the attributes

8e2e0ad

MarcinKasprowicz reviewed May 22, 2018

View reviewed changes

jlampar and others added 10 commits June 4, 2018 09:29

changing to redux way and reducing the mess

1596234

Merge branch 'release_2.0' into addHeatmap

daa5b3d

fixing imports

22ebea0

Merge branch 'addHeatmap' of https://github.com/Appliscale/xprof into…

f064018

… addHeatmap

fixing imports and line length

2f6b271

still fixing imports

d79246b

fixing MC being undefined

76d4281

reverting

8eb3ffb

fixing test to fit new return format

1cc4587

fixing indentation

d4247d1

jlampar added 6 commits June 5, 2018 16:07

fixing

44ee97b

fixing roll assignement and labels positioning

29f53da

experiment

ebe69ad

checking for data

39c9f49

resolving conflicts

c188f8c

adding label tooltip, id assignement

322b7c1

jlampar added 2 commits June 7, 2018 14:31

removing some comments

8c08420

changing the cursors

92d561c

MarcinKasprowicz reviewed Jun 11, 2018

View reviewed changes

MarcinKasprowicz approved these changes Jun 11, 2018

View reviewed changes

moving safecomposeID to state

bf6c4e9

gomoripeti approved these changes Jun 12, 2018

View reviewed changes

gomoripeti merged commit 4db675d into release_2.0 Jun 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding heatmap and graph_type #134

Adding heatmap and graph_type #134

jlampar commented May 8, 2018

jlampar commented May 9, 2018

jlampar commented May 9, 2018 •

edited

Loading

gomoripeti left a comment

gomoripeti May 9, 2018

gomoripeti May 9, 2018

jlampar May 10, 2018

gomoripeti May 9, 2018 •

edited

Loading

jlampar May 10, 2018 •

edited

Loading

gomoripeti May 10, 2018

amamla May 10, 2018

jlampar May 10, 2018 •

edited

Loading

MarcinKasprowicz May 21, 2018

MarcinKasprowicz May 21, 2018

MarcinKasprowicz May 21, 2018

MarcinKasprowicz May 21, 2018

MarcinKasprowicz May 21, 2018

MarcinKasprowicz May 21, 2018

jlampar May 23, 2018 •

edited

Loading

jlampar May 23, 2018

MarcinKasprowicz May 21, 2018

jlampar commented Jun 5, 2018

jlampar commented Jun 8, 2018

MarcinKasprowicz commented Jun 11, 2018

MarcinKasprowicz Jun 11, 2018

MarcinKasprowicz Jun 11, 2018

Adding heatmap and graph_type #134

Adding heatmap and graph_type #134

Conversation

jlampar commented May 8, 2018

jlampar commented May 9, 2018

jlampar commented May 9, 2018 • edited Loading

gomoripeti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gomoripeti May 9, 2018 • edited Loading

Choose a reason for hiding this comment

jlampar May 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlampar May 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlampar May 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlampar commented Jun 5, 2018

jlampar commented Jun 8, 2018

MarcinKasprowicz commented Jun 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlampar commented May 9, 2018 •

edited

Loading

gomoripeti May 9, 2018 •

edited

Loading

jlampar May 10, 2018 •

edited

Loading

jlampar May 10, 2018 •

edited

Loading

jlampar May 23, 2018 •

edited

Loading