elastic-charts icon indicating copy to clipboard operation
elastic-charts copied to clipboard

Flamegraph renders incorrectly on successive renders with different data

Open dgieselaar opened this issue 3 years ago • 3 comments
trafficstars

(copied from @jbcrail's issue in a private repo

Describe the bug

When the time range is updated for a flamegraph, we fetch the respective data; then the flamegraph will re-render without a page refresh. In the common case where the data before and after are different, then the flamegraph will render incorrectly (text will be offset from the expected rectangle bounds, colors will not applied to the rectangles, etc). At least in Firefox, we see an error like this:

WebGL warning: drawArraysInstanced: Instance fetch requires 15054, but attribs only supply 12822.

What's interesting is that the left number (15054) represents the length of the position array before the time range was updated, whereas 12822 is the array length after fetching.

To Reproduce

Steps to reproduce the behavior:

  1. Go to Flamegraphs section
  2. Select a time range larger than the existing range
  3. See error

See these screenshots that reproduce the behavior:

  • Flamegraph before updating time range

30 minutes

  • Flamegraph after updating time range (1st time)

24 hours

  • Flamegraph after updating time range (2nd time)

7 days

Expected behavior

The flamegraph should always rendering correctly when the time range is updated. No WebGL warnings should be seen in the browser console.

Additional context

I created a sample test case from the existing storybook story to demonstrate the issue. I have two different columnar datasets and the example toggles between the datasets every 10 seconds.

flamechart

/*
 * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
 * or more contributor license agreements. Licensed under the Elastic License
 * 2.0 and the Server Side Public License, v 1; you may not use this file except
 * in compliance with, at your election, the Elastic License 2.0 or the Server
 * Side Public License, v 1.
 */

import { action } from '@storybook/addon-actions';
import React, { useEffect, useState } from 'react';

import { Chart, ColumnarViewModel, Datum, Flame, Settings, PartialTheme, ControlProviderCallback } from '@elastic/charts';
import columnarMock from '@elastic/charts/src/mocks/hierarchical/cpu_profile_tree_mock_columnar.json';
import { getRandomNumberGenerator } from '@elastic/charts/src/mocks/utils';

import { useBaseTheme } from '../../use_base_theme';

const pseudoRandom = getRandomNumberGenerator('a_seed');

const paletteColorBrewerCat12 = [
  [141, 211, 199],
  [255, 255, 179],
  [190, 186, 218],
  [251, 128, 114],
  [128, 177, 211],
  [253, 180, 98],
  [179, 222, 105],
  [252, 205, 229],
  [217, 217, 217],
  [188, 128, 189],
  [204, 235, 197],
  [255, 237, 111],
];

function generateColorsForLabels(labels: any[]): number[] {
  return labels.flatMap(() => [...paletteColorBrewerCat12[pseudoRandom(0, 11)].map((c) => c / 255), 1]);
}

const largeDataset: ColumnarViewModel = (() => {
  const labels = columnarMock.label;
  const value = new Float64Array(columnarMock.value);
  const position = new Float32Array(columnarMock.position);
  const size = new Float32Array(columnarMock.size);

  return {
    label: labels.map((index: number) => columnarMock.dictionary[index]),
    value: value,
    color: new Float32Array(generateColorsForLabels(labels)),
    position0: position,
    position1: position,
    size0: size,
    size1: size,
  };
})();

const smallDataset: ColumnarViewModel = (() => {
  const labels = ["root", "kernel", "libxul.so", "libxul.so"];
  const value = [1428, 1428, 1099, 329];
  const position = [0, 0.67, 0, 0.33, 0, 0, 0.769607, 0];
  const size = [1, 1, 0.769607, 0.230393];

  return {
    label: labels,
    value: new Float64Array(value),
    color: new Float32Array(generateColorsForLabels(labels)),
    position0: new Float32Array(position),
    position1: new Float32Array(position),
    size0: new Float32Array(size),
    size1: new Float32Array(size),
  };
})();

export const Example = (
  // should check why it's not a good idea; in the meantime:
  // eslint-disable-next-line unicorn/no-object-as-default-parameter
  props?: { controlProviderCallback: ControlProviderCallback },
) => {
  const [columnarData, setColumnarData] = useState(largeDataset);
  const [seconds, setSeconds] = useState(0);

  const onElementListeners = {
    onElementClick: action('onElementClick'),
    onElementOver: action('onElementOver'),
    onElementOut: action('onElementOut'),
  };

  const theme: PartialTheme = {
    chartMargins: { top: 0, left: 0, bottom: 0, right: 0 },
    chartPaddings: { left: 0, right: 0, top: 0, bottom: 0 },
  };

  useEffect(() => {
    const interval = setInterval(() => {
      setSeconds(seconds => seconds + 1);
    }, 1000);
    return () => clearInterval(interval);
  }, []);

  useEffect(() => {
    if (seconds > 0 && seconds % 10 === 0) {
      if ((seconds / 10) % 2 == 0) {
        setColumnarData(largeDataset)
      } else {
        setColumnarData(smallDataset)
      }
    }
  }, [seconds]);

  return (
    <Chart>
      <Settings theme={theme} baseTheme={useBaseTheme()} {...onElementListeners} />
      <Flame
        id="spec_1"
        columnarData={columnarData}
        valueAccessor={(d: Datum) => d.value as number}
        valueFormatter={(value) => `${value}`}
        animation={{ duration: 500 }}
        controlProviderCallback={props?.controlProviderCallback ?? (() => { })}
      />
    </Chart>
  );
};

Example.parameters = {
  background: { default: 'white' },
};

dgieselaar avatar Jul 29 '22 12:07 dgieselaar

Thanks for the detailed report! We're looking into it. In the meantime, to avoid being blocked, would a fresh instantiation of the chart work? Ie. it gets a new key, so the old chart and its resources are removed from the DOM tree and a fresh one gets inserted

monfera avatar Aug 05 '22 22:08 monfera

@monfera I'm adding a key prop here: https://github.com/elastic/kibana/pull/138498. IMHO that's good enough for now.

dgieselaar avatar Aug 10 '22 11:08 dgieselaar

referencing original issue: https://github.com/elastic/prodfiler/issues/2460

timductive avatar Aug 31 '22 17:08 timductive

Closing as a workaround is already used. Please reopen in case you need an elastic-charts side fix

markov00 avatar Jan 29 '24 17:01 markov00