Skip to content

Commit

Permalink
[8.11] [Profiling] Fix Diff TopN bug (elastic#169549) (elastic#169648)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.11`:
- [[Profiling] Fix Diff TopN bug
(elastic#169549)](elastic#169549)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Cauê
Marcondes","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-24T12:53:14Z","message":"[Profiling]
Fix Diff TopN bug (elastic#169549)\n\nFixing this error that was happening when
hovering over the grid.\r\n<img width=\"1637\" alt=\"Screenshot
2023-10-23 at 17 14
26\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/5baf2679-6803-4576-bb75-a52b41a902a0\">","sha":"0a25b39df25b61537c7c5aec9c441195f99845fd","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v8.11.0","v8.12.0","v8.10.5"],"number":169549,"url":"https://github.com/elastic/kibana/pull/169549","mergeCommit":{"message":"[Profiling]
Fix Diff TopN bug (elastic#169549)\n\nFixing this error that was happening when
hovering over the grid.\r\n<img width=\"1637\" alt=\"Screenshot
2023-10-23 at 17 14
26\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/5baf2679-6803-4576-bb75-a52b41a902a0\">","sha":"0a25b39df25b61537c7c5aec9c441195f99845fd"}},"sourceBranch":"main","suggestedTargetBranches":["8.11","8.10"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/169549","number":169549,"mergeCommit":{"message":"[Profiling]
Fix Diff TopN bug (elastic#169549)\n\nFixing this error that was happening when
hovering over the grid.\r\n<img width=\"1637\" alt=\"Screenshot
2023-10-23 at 17 14
26\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/5baf2679-6803-4576-bb75-a52b41a902a0\">","sha":"0a25b39df25b61537c7c5aec9c441195f99845fd"}},{"branch":"8.10","label":"v8.10.5","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Cauê Marcondes <[email protected]>
  • Loading branch information
kibanamachine and cauemarcondes authored Oct 24, 2023
1 parent 43ad7c6 commit 34f92a2
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 37 deletions.
6 changes: 3 additions & 3 deletions x-pack/plugins/observability/server/ui_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ export const uiSettings: Record<string, UiSettings> = {
defaultMessage: `The average amortized per-core power consumption (based on 100% CPU utilization).`,
}),
schema: schema.number({ min: 0 }),
requiresPageReload: false,
requiresPageReload: true,
},
[profilingDatacenterPUE]: {
category: [observabilityFeatureId],
Expand Down Expand Up @@ -425,7 +425,7 @@ export const uiSettings: Record<string, UiSettings> = {
},
}),
schema: schema.number({ min: 0 }),
requiresPageReload: false,
requiresPageReload: true,
},
[profilingCo2PerKWH]: {
category: [observabilityFeatureId],
Expand All @@ -448,7 +448,7 @@ export const uiSettings: Record<string, UiSettings> = {
},
}),
schema: schema.number({ min: 0 }),
requiresPageReload: false,
requiresPageReload: true,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ describe.skip('Functions page', () => {
.clear()
.type('20');
cy.contains('Save changes').click();
cy.getByTestSubj('kbnLoadingMessage').should('exist');
cy.getByTestSubj('kbnLoadingMessage').should('not.exist', {
timeout: 50000,
});
cy.go('back');
cy.wait('@getTopNFunctions');
cy.get(firstRowSelector).eq(5).contains('24.22k lbs / 10.99k');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import {
useEuiTheme,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { TopNFunctionSortField } from '@kbn/profiling-utils';
import React, { useEffect } from 'react';
import { asCost } from '../../utils/formatters/as_cost';
import { asWeight } from '../../utils/formatters/as_weight';
import { StackFrameSummary } from '../stack_frame_summary';
Expand All @@ -39,38 +39,8 @@ export function FunctionRow({
onFrameClick,
setCellProps,
}: Props) {
const theme = useEuiTheme();
const successColor = useEuiBackgroundColor('success');
const dangerColor = useEuiBackgroundColor('danger');

if (columnId === TopNFunctionSortField.Diff) {
if (!functionRow.diff) {
return (
<EuiText size="xs" color={theme.euiTheme.colors.primaryText}>
{i18n.translate('xpack.profiling.functionsView.newLabel', {
defaultMessage: 'New',
})}
</EuiText>
);
}

if (functionRow.diff.rank === 0) {
return null;
}

const color = functionRow.diff.rank > 0 ? 'success' : 'danger';
setCellProps({ style: { backgroundColor: color === 'success' ? successColor : dangerColor } });

return (
<EuiFlexGroup direction="row" gutterSize="xs">
<EuiFlexItem grow={false}>
<EuiIcon type={functionRow.diff.rank > 0 ? 'sortUp' : 'sortDown'} color={color} />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText size="s">{Math.abs(functionRow.diff.rank)}</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
);
return <DiffColumn diff={functionRow.diff} setCellProps={setCellProps} />;
}

if (columnId === TopNFunctionSortField.Rank) {
Expand All @@ -82,12 +52,12 @@ export function FunctionRow({
}

if (columnId === TopNFunctionSortField.Samples) {
setCellProps({ css: { textAlign: 'right' } });
return (
<SampleStat
<SamplesColumn
samples={functionRow.samples}
diffSamples={functionRow.diff?.samples}
totalSamples={totalCount}
setCellProps={setCellProps}
/>
);
}
Expand Down Expand Up @@ -116,3 +86,65 @@ export function FunctionRow({

return null;
}

interface SamplesColumnProps {
samples: number;
diffSamples?: number;
totalSamples: number;
setCellProps: EuiDataGridCellValueElementProps['setCellProps'];
}

function SamplesColumn({ samples, totalSamples, diffSamples, setCellProps }: SamplesColumnProps) {
useEffect(() => {
setCellProps({ css: { textAlign: 'right' } });
}, [setCellProps]);
return <SampleStat samples={samples} diffSamples={diffSamples} totalSamples={totalSamples} />;
}

interface DiffColumnProps {
diff: IFunctionRow['diff'];
setCellProps: EuiDataGridCellValueElementProps['setCellProps'];
}

function DiffColumn({ diff, setCellProps }: DiffColumnProps) {
const theme = useEuiTheme();
const successColor = useEuiBackgroundColor('success');
const dangerColor = useEuiBackgroundColor('danger');

useEffect(() => {
if (diff && diff.rank > 0) {
const color = diff.rank > 0 ? 'success' : 'danger';
setCellProps({
style: { backgroundColor: color === 'success' ? successColor : dangerColor },
});
}
}, [dangerColor, diff, setCellProps, successColor]);

if (!diff) {
return (
<EuiText size="xs" color={theme.euiTheme.colors.primaryText}>
{i18n.translate('xpack.profiling.functionsView.newLabel', {
defaultMessage: 'New',
})}
</EuiText>
);
}

if (diff.rank === 0) {
return null;
}

return (
<EuiFlexGroup direction="row" gutterSize="xs">
<EuiFlexItem grow={false}>
<EuiIcon
type={diff.rank > 0 ? 'sortUp' : 'sortDown'}
color={diff.rank > 0 ? 'success' : 'danger'}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText size="s">{Math.abs(diff.rank)}</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
);
}

0 comments on commit 34f92a2

Please sign in to comment.