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

Fix issues with PerformanceReport Page (commit data, hiding missing data) #1461

Open
wants to merge 1 commit into
base: performance-benchmark-page
Choose a base branch
from
Open

Fix issues with PerformanceReport Page (commit data, hiding missing data) #1461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/common/Axios.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import axios from 'axios';

export const AxiosConfig = axios.create({
baseURL: process.env.REACT_APP_LAMBDA_URL,
timeout: 3000,
timeout: 5000,
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this based on can we document it? can we also document that this is in milli seconds

Copy link
Author

Choose a reason for hiding this comment

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

The trends page often runs into a timeout when collecting commit data.

Will update this to add retry/show an error page as well.

headers: {
'Content-Type': 'application/json',
},
Expand Down
2 changes: 2 additions & 0 deletions src/containers/PerformanceReport/data.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export interface PerformanceMetricReport {
};

UseCase: { S: string };
Service: { S: string };
UniqueID: { S: string };
}

// PerformanceMetric shows all collected metrics when running performance metrics
Expand Down
37 changes: 16 additions & 21 deletions src/containers/PerformanceReport/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,22 @@ import { TRANSACTION_PER_MINUTE } from '../../common/Constant';
import { usePageEffect } from '../../core/page';
import { PerformanceTable } from '../../core/table';
import { UseCaseData } from './data';
import { GetLatestPerformanceReports, GetServiceLatestVersion } from './service';
import { GetLatestPerformanceReports, GetServiceLatestVersion, GetServicePRInformation } from './service';
import { PasswordDialog } from '../../common/Dialog';
import { SelectChangeEvent } from '@mui/material/Select';

export default function PerformanceReport(props: { password: string; password_is_set: boolean; set_password_state: any }): JSX.Element {
usePageEffect({ title: 'Amazon CloudWatch Agent' });
const { password, password_is_set, set_password_state } = props;
const [{ version, commit_date, commit_title, commit_hash, commit_url, use_cases, ami_id, collection_period }] = useStatePerformanceReport(password);
const [{ data_type }, setDataTypeState] = useStateDataType();

const handleDataTypeChange = (event: SelectChangeEvent) => {
setDataTypeState({ data_type: event.target.value });
};

const selectedUseCaseData: UseCaseData[] = use_cases.filter((useCase: UseCaseData) => useCase?.data_type?.toLowerCase() === data_type.toLowerCase());

return (
<Container>
<PasswordDialog password={password} password_is_set={password_is_set} set_password_state={set_password_state} />
Expand Down Expand Up @@ -89,19 +97,7 @@ export default function PerformanceReport(props: { password: string; password_is
) : name === 'Commit Date' ? (
<Typography variant="h4">{commit_date}</Typography>
) : (
<Select
sx={{ height: '41px' }}
value={data_type}
onChange={(e: {
target: {
value: string;
};
}) =>
setDataTypeState({
data_type: e.target.value,
})
}
>
<Select sx={{ height: '41px' }} value={data_type} onChange={handleDataTypeChange}>
<MenuItem value={'Metrics'}>Metric</MenuItem>
<MenuItem value={'Traces'}>Trace</MenuItem>
<MenuItem value={'Logs'}>Logs</MenuItem>
Expand All @@ -120,7 +116,7 @@ export default function PerformanceReport(props: { password: string; password_is
<Typography sx={{ mb: 2, fontWeight: 'bold' }} variant="h3">
{data_type} (TPM: {tpm}){' '}
</Typography>
<PerformanceTable data_rate={String(tpm)} use_cases={use_cases.filter((use_case: UseCaseData) => use_case?.data_type === data_type.toLowerCase())} />
<PerformanceTable key={data_type} data_rate={String(tpm)} use_cases={selectedUseCaseData} />
</Container>
))}
</Container>
Expand Down Expand Up @@ -166,7 +162,7 @@ function useStatePerformanceReport(password: string) {
name: pReport?.UseCase.S,
data_type: pReport?.DataType.S,
instance_type: pReport?.InstanceType.S,
data: TRANSACTION_PER_MINUTE.reduce(
data: Object.keys(pReport?.Results.M).reduce(
(accu, tpm) => ({
...accu,
[tpm]: {
Expand All @@ -187,19 +183,18 @@ function useStatePerformanceReport(password: string) {
),
});
}
// const commit_info = await GetServicePRInformation(password, commit_hash);

const commit_info = await GetServicePRInformation(password, commit_hash);

setState((prev: any) => ({
...prev,
version: service_info.tag_name,
ami_id: ami_id,
collection_period: collection_period,
use_cases: use_cases,
// commit_title: `${commit_info?.title} (#${commit_info?.number})`,
// commit_url: commit_info?.html_url,
commit_title: `${commit_info?.title} (#${commit_info?.number})`,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add default fallbacks here to make sure it doesn't break

commit_url: commit_info?.html_url,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add default fallbacks here to make sure it doesn't break

commit_hash: commit_hash,
commit_title: `PlaceHolder`,
commit_url: `www.github.com/aws/amazon-cloudwatch-agent`,
commit_date: moment.unix(Number(commit_date)).format('dddd, MMMM Do, YYYY h:mm:ss A'),
}));
})();
Expand Down
8 changes: 6 additions & 2 deletions src/containers/PerformanceTrend/data.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,13 @@ export interface ServiceCommitInformation {
// Release version for the service
author: {
login: string;
date: string;
};
commit: { message: string };
commit: {
message: string;
author: {
date: string;
};
};
sha: string;
}

Expand Down
4 changes: 2 additions & 2 deletions src/containers/PerformanceTrend/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,10 @@ export default function PerformanceTrend(props: { password: string; password_is_
const selected_data = series[seriesIndex][dataPointIndex];
const selected_hash = w.globals.categoryLabels[dataPointIndex];
const selected_hash_information: ServiceCommitInformation | undefined = commits_information
.filter((c: ServiceCommitInformation) => c.sha === selected_hash)
.filter((c: ServiceCommitInformation) => c.sha.startsWith(selected_hash))
.at(0);
const commit_history = selected_hash_information?.commit.message.replace(/\n\r*\n*/g, '<br />');
const commited_by = `Committed by ${selected_hash_information?.author.login} on ${selected_hash_information?.author.date}`;
const commited_by = `Committed by ${selected_hash_information?.author.login} on ${selected_hash_information?.commit?.author?.date}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add default fallbacks here to make sure it doesn't break

const commit_data = `<b>${use_case}</b>: ${selected_data}`;

return (
Expand Down
32 changes: 17 additions & 15 deletions src/core/table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,21 +154,23 @@ export function PerformanceTable(props: { use_cases: UseCaseData[]; data_rate: s
</TableRow>
</TableHead>
<TableBody>
{use_cases?.map((use_case) => (
<StyledTableRow key={use_case.name}>
<StyledTableCell>{use_case.name}</StyledTableCell>
<StyledTableCell>{use_case.instance_type}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.procstat_cpu_usage).toFixed(2)}</StyledTableCell>
<StyledTableCell>{(Number(use_case.data?.[data_rate]?.procstat_memory_rss) / Number(use_case.data?.[data_rate]?.mem_total)).toFixed(2)}</StyledTableCell>
<StyledTableCell>{(Number(use_case.data?.[data_rate]?.procstat_memory_swap) / Number(use_case.data?.[data_rate]?.mem_total)).toFixed(2)}</StyledTableCell>
<StyledTableCell>{(Number(use_case.data?.[data_rate]?.procstat_memory_data) / Number(use_case.data?.[data_rate]?.mem_total)).toFixed(2)}</StyledTableCell>
<StyledTableCell>{(Number(use_case.data?.[data_rate]?.procstat_memory_vms) / Number(use_case.data?.[data_rate]?.mem_total)).toFixed(2)}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.procstat_write_bytes).toFixed(2)}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.procstat_num_fds).toFixed(2)}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.net_bytes_sent).toFixed(2)}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.net_packets_sent).toFixed(2)}</StyledTableCell>
</StyledTableRow>
))}
{use_cases?.map((use_case: UseCaseData) => {
return use_case.data?.[data_rate] ? (
<StyledTableRow key={use_case.name}>
<StyledTableCell>{use_case.name}</StyledTableCell>
<StyledTableCell>{use_case.instance_type}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.procstat_cpu_usage).toFixed(2)}</StyledTableCell>
<StyledTableCell>{(Number(use_case.data?.[data_rate]?.procstat_memory_rss) / Number(use_case.data?.[data_rate]?.mem_total)).toFixed(2)}</StyledTableCell>
<StyledTableCell>{(Number(use_case.data?.[data_rate]?.procstat_memory_swap) / Number(use_case.data?.[data_rate]?.mem_total)).toFixed(2)}</StyledTableCell>
<StyledTableCell>{(Number(use_case.data?.[data_rate]?.procstat_memory_data) / Number(use_case.data?.[data_rate]?.mem_total)).toFixed(2)}</StyledTableCell>
<StyledTableCell>{(Number(use_case.data?.[data_rate]?.procstat_memory_vms) / Number(use_case.data?.[data_rate]?.mem_total)).toFixed(2)}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.procstat_write_bytes).toFixed(2)}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.procstat_num_fds).toFixed(2)}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.net_bytes_sent).toFixed(2)}</StyledTableCell>
<StyledTableCell>{Number(use_case.data?.[data_rate]?.net_packets_sent).toFixed(2)}</StyledTableCell>
</StyledTableRow>
) : null;
Copy link
Contributor

Choose a reason for hiding this comment

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

is it better to return null than <> component ?

})}
</TableBody>
</Table>
</TableContainer>
Expand Down
Loading