Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Profiling] Adding normalize by time and scale factor on Diff TopN functions page #159394

Merged
merged 11 commits into from Jun 14, 2023
Merged

[Profiling] Adding normalize by time and scale factor on Diff TopN functions page #159394

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9927c52
Refactoring normalization menu
cauemarcondes Jun 7, 2023
9cf53b9
new comp
cauemarcondes Jun 8, 2023
b13e3d8
refactoring differential_comparison_mode component
cauemarcondes Jun 8, 2023
b667deb
refactoring functions view
cauemarcondes Jun 9, 2023
c4b4f76
adding default options
cauemarcondes Jun 9, 2023
f7ec129
scaling samples
cauemarcondes Jun 9, 2023
c6e840d
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jun 9, 2023
3777f35
fixing i18n
cauemarcondes Jun 12, 2023
93c9885
Merge branch 'profiling-functions-normalize-by' of github.com:cauemar…
cauemarcondes Jun 12, 2023
3593f54
Merge branch 'main' into profiling-functions-normalize-by
cauemarcondes Jun 12, 2023
dd9aed7
Merge branch 'main' into profiling-functions-normalize-by
cauemarcondes Jun 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 0 additions & 10 deletions x-pack/plugins/profiling/common/flamegraph.ts
View file
Open in desktop
Expand Up @@ -10,16 +10,6 @@ import { createFrameGroupID } from './frame_group';
import { fnv1a64 } from './hash';
import { createStackFrameMetadata, getCalleeLabel } from './profiling';

export enum FlameGraphComparisonMode {
Absolute = 'absolute',
Relative = 'relative',
}

export enum FlameGraphNormalizationMode {
Scale = 'scale',
Time = 'time',
}

export interface BaseFlameGraph {
Size: number;
Edges: number[][];
Expand Down
42 changes: 17 additions & 25 deletions ...phs_view/differential_comparison_mode.tsx → ...ts/differential_comparison_mode/index.tsx
View file
Open in desktop
Expand Up @@ -7,11 +7,11 @@
import { EuiButtonGroup, EuiFlexGroup, EuiFlexItem, EuiTitle } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { FlameGraphComparisonMode } from '../../../common/flamegraph';
import { ComparisonMode } from '../normalization_menu';

interface Props {
comparisonMode: FlameGraphComparisonMode;
onChange: (nextComparisonMode: FlameGraphComparisonMode) => void;
comparisonMode: ComparisonMode;
onChange: (nextComparisonMode: ComparisonMode) => void;
}
export function DifferentialComparisonMode({ comparisonMode, onChange }: Props) {
return (
Expand All @@ -20,45 +20,37 @@ export function DifferentialComparisonMode({ comparisonMode, onChange }: Props)
<EuiFlexItem grow={false}>
<EuiTitle size="xxs">
<h3>
{i18n.translate(
'xpack.profiling.flameGraphsView.differentialFlameGraphComparisonModeTitle',
{ defaultMessage: 'Format' }
)}
{i18n.translate('xpack.profiling.differentialComparisonMode.title', {
defaultMessage: 'Format',
})}
</h3>
</EuiTitle>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonGroup
legend={i18n.translate(
'xpack.profiling.flameGraphsView.differentialFlameGraphComparisonModeLegend',
{
defaultMessage:
'This switch allows you to switch between an absolute and relative comparison between both graphs',
}
)}
legend={i18n.translate('xpack.profiling.differentialComparisonMode.legend', {
defaultMessage:
'This switch allows you to switch between an absolute and relative comparison between both graphs',
})}
type="single"
buttonSize="s"
idSelected={comparisonMode}
onChange={(nextComparisonMode) => {
onChange(nextComparisonMode as FlameGraphComparisonMode);
onChange(nextComparisonMode as ComparisonMode);
}}
options={[
{
id: FlameGraphComparisonMode.Absolute,
id: ComparisonMode.Absolute,
label: i18n.translate(
'xpack.profiling.flameGraphsView.differentialFlameGraphComparisonModeAbsoluteButtonLabel',
{
defaultMessage: 'Abs',
}
'xpack.profiling.differentialComparisonMode.absoluteButtonLabel',
{ defaultMessage: 'Abs' }
),
},
{
id: FlameGraphComparisonMode.Relative,
id: ComparisonMode.Relative,
label: i18n.translate(
'xpack.profiling.flameGraphsView.differentialFlameGraphComparisonModeRelativeButtonLabel',
{
defaultMessage: 'Rel',
}
'xpack.profiling.differentialComparisonMode.relativeButtonLabel',
{ defaultMessage: 'Rel' }
),
},
]}
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/profiling/public/components/flamegraph/index.tsx
View file
Open in desktop
Expand Up @@ -9,16 +9,17 @@ import { Chart, Datum, Flame, FlameLayerValue, PartialTheme, Settings } from '@e
import { EuiFlexGroup, EuiFlexItem, useEuiTheme } from '@elastic/eui';
import { Maybe } from '@kbn/observability-plugin/common/typings';
import React, { useEffect, useMemo, useState } from 'react';
import { ElasticFlameGraph, FlameGraphComparisonMode } from '../../../common/flamegraph';
import { ElasticFlameGraph } from '../../../common/flamegraph';
import { getFlamegraphModel } from '../../utils/get_flamegraph_model';
import { FlameGraphLegend } from './flame_graph_legend';
import { FrameInformationWindow } from '../frame_information_window';
import { FrameInformationTooltip } from '../frame_information_window/frame_information_tooltip';
import { FlameGraphTooltip } from './flamegraph_tooltip';
import { ComparisonMode } from '../normalization_menu';

interface Props {
id: string;
comparisonMode: FlameGraphComparisonMode;
comparisonMode: ComparisonMode;
primaryFlamegraph?: ElasticFlameGraph;
comparisonFlamegraph?: ElasticFlameGraph;
baseline?: number;
Expand Down
74 changes: 37 additions & 37 deletions .../flame_graphs_view/normalization_menu.tsx → ...c/components/normalization_menu/index.tsx
View file
Open in desktop
Expand Up @@ -25,35 +25,41 @@ import {
import { css } from '@emotion/react';
import { i18n } from '@kbn/i18n';
import React, { useEffect, useState } from 'react';
import { FlameGraphNormalizationMode } from '../../../common/flamegraph';

export interface FlameGraphNormalizationOptions {
export interface NormalizationOptions {
baselineScale: number;
baselineTime: number;
comparisonScale: number;
comparisonTime: number;
}

export enum ComparisonMode {
Absolute = 'absolute',
Relative = 'relative',
}

export enum NormalizationMode {
Scale = 'scale',
Time = 'time',
}

cauemarcondes marked this conversation as resolved.
Show resolved Hide resolved
interface Props {
mode: FlameGraphNormalizationMode;
options: FlameGraphNormalizationOptions;
onChange: (mode: FlameGraphNormalizationMode, options: FlameGraphNormalizationOptions) => void;
mode: NormalizationMode;
options: NormalizationOptions;
onChange: (mode: NormalizationMode, options: NormalizationOptions) => void;
}

const SCALE_LABEL = i18n.translate('xpack.profiling.flameGraphNormalizationMenu.scale', {
const SCALE_LABEL = i18n.translate('xpack.profiling.normalizationMenu.scale', {
defaultMessage: 'Scale factor',
});

const TIME_LABEL = i18n.translate('xpack.profiling.flameGraphNormalizationMenu.time', {
const TIME_LABEL = i18n.translate('xpack.profiling.normalizationMenu.time', {
defaultMessage: 'Time',
});

const NORMALIZE_BY_LABEL = i18n.translate(
'xpack.profiling.flameGraphNormalizationMenu.normalizeBy',
{
defaultMessage: 'Normalize by',
}
);
const NORMALIZE_BY_LABEL = i18n.translate('xpack.profiling.normalizationMenu.normalizeBy', {
defaultMessage: 'Normalize by',
});

export function NormalizationMenu(props: Props) {
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
Expand All @@ -72,7 +78,7 @@ export function NormalizationMenu(props: Props) {
}, [props.mode, props.options]);

const { baseline, comparison } =
mode === FlameGraphNormalizationMode.Time
mode === NormalizationMode.Time
? { comparison: options.comparisonTime, baseline: options.baselineTime }
: { comparison: options.comparisonScale, baseline: options.baselineScale };

Expand Down Expand Up @@ -110,7 +116,7 @@ export function NormalizationMenu(props: Props) {
padding: '0 16px',
}}
>
{props.mode === FlameGraphNormalizationMode.Scale ? SCALE_LABEL : TIME_LABEL}
{props.mode === NormalizationMode.Scale ? SCALE_LABEL : TIME_LABEL}
</EuiFlexItem>
</EuiFormControlLayout>
}
Expand All @@ -129,24 +135,18 @@ export function NormalizationMenu(props: Props) {
<EuiFlexGroup direction="column" gutterSize="s">
<EuiFlexItem>
<span>
{i18n.translate(
'xpack.profiling.flameGraphNormalizationMenu.normalizeByTimeTooltip',
{
defaultMessage:
'Select Normalize by Scale factor and set your Baseline and Comparison scale factors to compare a set of machines of different sizes. For example, you can compare a deployment of 10% of machines to a deployment of 90% of machines.',
}
)}
{i18n.translate('xpack.profiling.normalizationMenu.normalizeByTimeTooltip', {
defaultMessage:
'Select Normalize by Scale factor and set your Baseline and Comparison scale factors to compare a set of machines of different sizes. For example, you can compare a deployment of 10% of machines to a deployment of 90% of machines.',
})}
</span>
</EuiFlexItem>
<EuiFlexItem>
<span>
{i18n.translate(
'xpack.profiling.flameGraphNormalizationMenu.normalizeByScaleTooltip',
{
defaultMessage:
'Select Normalize by Time to compare a set of machines across different time periods. For example, if you compare the last hour to the last 24 hours, the shorter timeframe (1 hour) is multiplied to match the longer timeframe (24 hours).',
}
)}
{i18n.translate('xpack.profiling.normalizationMenu.normalizeByScaleTooltip', {
defaultMessage:
'Select Normalize by Time to compare a set of machines across different time periods. For example, if you compare the last hour to the last 24 hours, the shorter timeframe (1 hour) is multiplied to match the longer timeframe (24 hours).',
})}
</span>
</EuiFlexItem>
</EuiFlexGroup>
Expand All @@ -160,19 +160,19 @@ export function NormalizationMenu(props: Props) {
buttonSize="compressed"
isFullWidth
onChange={(id, value) => {
setMode(id as FlameGraphNormalizationMode);
setMode(id as NormalizationMode);
}}
legend={i18n.translate('xpack.profiling.flameGraphNormalizationMode.selectModeLegend', {
legend={i18n.translate('xpack.profiling.normalizationMode.selectModeLegend', {
defaultMessage: 'Select a normalization mode for the flamegraph',
})}
idSelected={mode}
options={[
{
id: FlameGraphNormalizationMode.Scale,
id: NormalizationMode.Scale,
label: SCALE_LABEL,
},
{
id: FlameGraphNormalizationMode.Time,
id: NormalizationMode.Time,
label: TIME_LABEL,
},
]}
Expand All @@ -195,14 +195,14 @@ export function NormalizationMenu(props: Props) {
id={baselineScaleFactorInputId}
value={baseline}
onChange={(e) => {
if (mode === FlameGraphNormalizationMode.Scale) {
if (mode === NormalizationMode.Scale) {
setOptions((prevOptions) => ({
...prevOptions,
baselineScale: e.target.valueAsNumber,
}));
}
}}
disabled={mode === FlameGraphNormalizationMode.Time}
disabled={mode === NormalizationMode.Time}
/>
</EuiFormControlLayout>
<EuiSpacer size="m" />
Expand All @@ -223,14 +223,14 @@ export function NormalizationMenu(props: Props) {
id={comparisonScaleFactorInputId}
value={comparison}
onChange={(e) => {
if (mode === FlameGraphNormalizationMode.Scale) {
if (mode === NormalizationMode.Scale) {
setOptions((prevOptions) => ({
...prevOptions,
comparisonScale: e.target.valueAsNumber,
}));
}
}}
disabled={mode === FlameGraphNormalizationMode.Time}
disabled={mode === NormalizationMode.Time}
/>
</EuiFormControlLayout>
<EuiSpacer size="m" />
Expand Down
59 changes: 42 additions & 17 deletions x-pack/plugins/profiling/public/components/topn_functions/index.tsx
View file
Open in desktop
Expand Up @@ -92,7 +92,7 @@ function SampleStat({
diffSamples?: number;
totalSamples: number;
}) {
const samplesLabel = `${samples.toLocaleString()}`;
const samplesLabel = samples.toLocaleString();

if (diffSamples === undefined || diffSamples === 0 || totalSamples === 0) {
return <>{samplesLabel}</>;
Expand Down Expand Up @@ -142,6 +142,12 @@ interface Props {
comparisonTopNFunctions?: TopNFunctions;
totalSeconds: number;
isDifferentialView: boolean;
baselineScaleFactor?: number;
comparisonScaleFactor?: number;
}

function scaleValue({ value, scaleFactor = 1 }: { value: number; scaleFactor?: number }) {
return value * scaleFactor;
}

export function TopNFunctionsTable({
Expand All @@ -152,6 +158,8 @@ export function TopNFunctionsTable({
comparisonTopNFunctions,
totalSeconds,
isDifferentialView,
baselineScaleFactor,
comparisonScaleFactor,
}: Props) {
const [selectedRow, setSelectedRow] = useState<Row | undefined>();

Expand All @@ -175,6 +183,11 @@ export function TopNFunctionsTable({
return topNFunctions.TopN.filter((topN) => topN.CountExclusive > 0).map((topN, i) => {
const comparisonRow = comparisonDataById?.[topN.Id];

const topNCountExclusiveScaled = scaleValue({
value: topN.CountExclusive,
scaleFactor: baselineScaleFactor,
});

const inclusiveCPU = (topN.CountInclusive / topNFunctions.TotalCount) * 100;
const exclusiveCPU = (topN.CountExclusive / topNFunctions.TotalCount) * 100;
const totalSamples = topN.CountExclusive;
Expand All @@ -189,31 +202,43 @@ export function TopNFunctionsTable({
})
: undefined;

const diff =
comparisonTopNFunctions && comparisonRow
? {
rank: topN.Rank - comparisonRow.Rank,
samples: topN.CountExclusive - comparisonRow.CountExclusive,
exclusiveCPU:
exclusiveCPU -
(comparisonRow.CountExclusive / comparisonTopNFunctions.TotalCount) * 100,
inclusiveCPU:
inclusiveCPU -
(comparisonRow.CountInclusive / comparisonTopNFunctions.TotalCount) * 100,
}
: undefined;
function calculateDiff() {
if (comparisonTopNFunctions && comparisonRow) {
const comparisonCountExclusiveScaled = scaleValue({
value: comparisonRow.CountExclusive,
scaleFactor: comparisonScaleFactor,
});

return {
rank: topN.Rank - comparisonRow.Rank,
samples: topNCountExclusiveScaled - comparisonCountExclusiveScaled,
exclusiveCPU:
exclusiveCPU -
(comparisonRow.CountExclusive / comparisonTopNFunctions.TotalCount) * 100,
inclusiveCPU:
inclusiveCPU -
(comparisonRow.CountInclusive / comparisonTopNFunctions.TotalCount) * 100,
Comment on lines +215 to +220
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: I'm not sure if I should scale the CPU values too! @elastic/profiling-ui does anybody know it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that scaling the CPU values would require scaling both the numerator and denominator, which would cancel out the scaling factor when dividing. So I don't believe it's necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To show my work and prove it to myself also, here's my reasoning:

  • All definitions apply equally to the inclusive and exclusive CPU values.
  • We'll assume inclusive without loss of generality for any CPUn and Totaln.
  • CPU1 is the baseline CPU
  • CPU2 is the comparison CPU
  • Total1 is the baseline total count
  • Total2 is the comparison total count
  • SF1 is the baseline scaling factor
  • SF2 is the comparison scaling factor

Without scaling

CPUdiff = 100 * (CPU1 / Total1) - 100 * (CPU2 / Total2)

or

CPUdiff = 100 * [(CPU1 / Total1) - (CPU2 / Total2)]

With scaling

CPUdiff = 100 * [(SF1* CPU1) / (SF1 * Total1)] - 100 * [(SF2 * CPU2) / (SF2 * Total2)]

or this after cancellation:

CPUdiff = 100 * (CPU1 / Total1) - 100 * (CPU2 / Total2)

which is the same as without scaling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL
wow, great explanation! thank you! 🤓

};
}
}

return {
rank: topN.Rank,
frame: topN.Frame,
samples: topN.CountExclusive,
samples: topNCountExclusiveScaled,
exclusiveCPU,
inclusiveCPU,
impactEstimates,
diff,
diff: calculateDiff(),
};
});
}, [topNFunctions, comparisonTopNFunctions, totalSeconds]);
}, [
topNFunctions,
comparisonTopNFunctions,
totalSeconds,
comparisonScaleFactor,
baselineScaleFactor,
]);

const theme = useEuiTheme();

Expand Down