Skip to content

Commit

Permalink
[Profiling] Fix UI discrepancies in frame information window (#142812)
Browse files Browse the repository at this point in the history
* Fix UI discrepancies in frame information window

* Fix comment

* Use source line instead

This matches with the value used in the flamegraph labels.
  • Loading branch information
jbcrail authored Oct 6, 2022
1 parent 2d34b6e commit 39811a3
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 28 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/profiling/common/profiling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export interface StackFrameMetadata {
// StackTrace.Type
FrameType: FrameType;

// StackFrame.LineNumber?
// StackTrace.AddressOrLine
AddressOrLine: number;
// StackFrame.FunctionName
FunctionName: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { NOT_AVAILABLE_LABEL } from '../../../common';
import { getImpactRows } from './get_impact_rows';
import { getInformationRows } from './get_information_rows';

interface Props {
frame?: {
fileID: string;
frameType: number;
exeFileName: string;
addressOrLine: number;
functionName: string;
sourceFileName: string;
sourceLine: number;
countInclusive: number;
countExclusive: number;
};
Expand Down Expand Up @@ -104,7 +108,27 @@ export function FlamegraphInformationWindow({ onClose, frame, totalSamples, tota
);
}

const { exeFileName, functionName, sourceFileName, countInclusive, countExclusive } = frame;
const {
fileID,
frameType,
exeFileName,
addressOrLine,
functionName,
sourceFileName,
sourceLine,
countInclusive,
countExclusive,
} = frame;

const informationRows = getInformationRows({
fileID,
frameType,
exeFileName,
addressOrLine,
functionName,
sourceFileName,
sourceLine,
});

const impactRows = getImpactRows({
countInclusive,
Expand All @@ -117,30 +141,7 @@ export function FlamegraphInformationWindow({ onClose, frame, totalSamples, tota
<FlamegraphFrameInformationPanel onClose={onClose}>
<EuiFlexGroup direction="column">
<EuiFlexItem>
<KeyValueList
rows={[
{
label: i18n.translate(
'xpack.profiling.flameGraphInformationWindow.executableLabel',
{ defaultMessage: 'Executable' }
),
value: exeFileName || NOT_AVAILABLE_LABEL,
},
{
label: i18n.translate('xpack.profiling.flameGraphInformationWindow.functionLabel', {
defaultMessage: 'Function',
}),
value: functionName || NOT_AVAILABLE_LABEL,
},
{
label: i18n.translate(
'xpack.profiling.flameGraphInformationWindow.sourceFileLabel',
{ defaultMessage: 'Source file' }
),
value: sourceFileName || NOT_AVAILABLE_LABEL,
},
]}
/>
<KeyValueList rows={informationRows} />
</EuiFlexItem>
<EuiFlexItem>
<EuiFlexGroup direction="column">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { i18n } from '@kbn/i18n';
import { NOT_AVAILABLE_LABEL } from '../../../common';
import { describeFrameType } from '../../../common/profiling';

export function getInformationRows({
fileID,
frameType,
exeFileName,
addressOrLine,
functionName,
sourceFileName,
sourceLine,
}: {
fileID: string;
frameType: number;
exeFileName: string;
addressOrLine: number;
functionName: string;
sourceFileName: string;
sourceLine: number;
}) {
const executable = fileID === '' && addressOrLine === 0 ? 'root' : exeFileName;
const sourceLineNumber = sourceLine > 0 ? `#${sourceLine}` : '';

const informationRows = [];

if (executable) {
informationRows.push({
label: i18n.translate('xpack.profiling.flameGraphInformationWindow.executableLabel', {
defaultMessage: 'Executable',
}),
value: executable,
});
} else {
informationRows.push({
label: i18n.translate('xpack.profiling.flameGraphInformationWindow.frameTypeLabel', {
defaultMessage: 'Frame type',
}),
value: describeFrameType(frameType),
});
}

informationRows.push({
label: i18n.translate('xpack.profiling.flameGraphInformationWindow.functionLabel', {
defaultMessage: 'Function',
}),
value: functionName || NOT_AVAILABLE_LABEL,
});

informationRows.push({
label: i18n.translate('xpack.profiling.flameGraphInformationWindow.sourceFileLabel', {
defaultMessage: 'Source file',
}),
value: sourceFileName ? `${sourceFileName}${sourceLineNumber}` : NOT_AVAILABLE_LABEL,
});

return informationRows;
}
6 changes: 5 additions & 1 deletion x-pack/plugins/profiling/public/components/flamegraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,13 @@ export const FlameGraph: React.FC<FlameGraphProps> = ({
const selected: undefined | React.ComponentProps<typeof FlamegraphInformationWindow>['frame'] =
primaryFlamegraph && highlightedVmIndex !== undefined
? {
fileID: primaryFlamegraph.FileID[highlightedVmIndex],
frameType: primaryFlamegraph.FrameType[highlightedVmIndex],
exeFileName: primaryFlamegraph.ExeFilename[highlightedVmIndex],
sourceFileName: primaryFlamegraph.SourceFilename[highlightedVmIndex],
addressOrLine: primaryFlamegraph.AddressOrLine[highlightedVmIndex],
functionName: primaryFlamegraph.FunctionName[highlightedVmIndex],
sourceFileName: primaryFlamegraph.SourceFilename[highlightedVmIndex],
sourceLine: primaryFlamegraph.SourceLine[highlightedVmIndex],
countInclusive: primaryFlamegraph.CountInclusive[highlightedVmIndex],
countExclusive: primaryFlamegraph.CountExclusive[highlightedVmIndex],
}
Expand Down

0 comments on commit 39811a3

Please sign in to comment.