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.

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

Metrics time selection #6360

Merged
merged 12 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
63 changes: 35 additions & 28 deletions frontend/src/components/HostMetricsDetail/Containers.styles.scss
Original file line number Diff line number Diff line change
@@ -1,34 +1,41 @@
.infra-container-card {
display: flex;
flex-direction: column;
justify-content: center;
}
.infra-container-card-text {
font-size: var(--font-size-sm);
color: var(--text-vanilla-400);
line-height: 20px;
letter-spacing: -0.07px;
width: 400px;
font-family: 'Inter';
}
.host-containers {
max-width: 600px;
margin: 150px auto;
padding: 0 16px;

.infra-container-card {
display: flex;
flex-direction: column;
justify-content: center;
}

.infra-container-working-msg {
display: flex;
width: 400px;
padding: 12px;
align-items: flex-start;
gap: 12px;
border-radius: 4px;
background: rgba(171, 189, 255, 0.04);
.infra-container-card-text {
font-size: var(--font-size-sm);
color: var(--text-vanilla-400);
line-height: 20px;
letter-spacing: -0.07px;
width: 400px;
font-family: 'Inter';
}

.ant-space {
.infra-container-working-msg {
display: flex;
width: 400px;
padding: 12px;
align-items: flex-start;
gap: 12px;
border-radius: 4px;
background: rgba(171, 189, 255, 0.04);

.ant-space {
align-items: flex-start;
}
}
}

.infra-container-contact-support-btn {
display: flex;
align-items: center;
justify-content: center;
margin: auto;
.infra-container-contact-support-btn {
display: flex;
align-items: center;
justify-content: center;
margin: auto;
}
}
6 changes: 1 addition & 5 deletions frontend/src/components/HostMetricsDetail/Containers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@ const { Text } = Typography;

function Containers(): JSX.Element {
return (
<Space
direction="vertical"
style={{ maxWidth: '600px', margin: '64px auto', padding: '0 16px' }}
size={24}
>
<Space direction="vertical" className="host-containers" size={24}>
<div className="infra-container-card">
<img
src="/Icons/infraContainers.svg"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ import { HostData } from 'api/infraMonitoring/getHostLists';

export type HostDetailProps = {
host: HostData | null;
isModalTimeSelection: boolean;
} & Pick<DrawerProps, 'onClose'>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
.host-metric-traces {
.ant-table {
background: #121212;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use design tokens or predefined color constants instead of hardcoding color values for background and color. This applies to lines 3, 4, 7, 8, 9, 13, 14, 15, 19, and 26.

Suggested change
background: #121212;
background: $background-color;

color: #fff;

.ant-table-thead>tr>th {
background: #121212;
color: #fff;
border-bottom: 1px solid #303030;
}

.ant-table-tbody>tr>td {
background: #121212;
color: #fff;
border-bottom: 1px solid #303030;
}

.ant-table-tbody>tr:hover>td {
background: #1a1a1a;
}
}

.duration-cell {
display: inline-block;
padding: 2px 8px;
background: #d64937;
border-radius: 4px;
color: #fff;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import './HostMetricTraces.styles.scss';

import { Skeleton } from 'antd';
import { ResizeTable } from 'components/ResizeTable';
import { DEFAULT_ENTITY_VERSION } from 'constants/app';
import { GetMetricQueryRange } from 'lib/dashboard/getQueryResults';
import { useEffect, useMemo, useState } from 'react';
import { useQuery } from 'react-query';

import { columns, getHostTracesQueryPayload } from './constants';

interface Props {
hostName: string;
timeRange: {
startTime: number;
endTime: number;
};
}

function HostMetricTraces({ hostName, timeRange }: Props): JSX.Element {
const [traces, setTraces] = useState<any[]>([]);
const [offset] = useState<number>(0);

const queryPayload = useMemo(
() =>
getHostTracesQueryPayload(
hostName,
timeRange.startTime,
timeRange.endTime,
offset,
),
[hostName, timeRange.startTime, timeRange.endTime, offset],
);

const { data, isLoading } = useQuery({
queryKey: ['hostMetricTraces', queryPayload, DEFAULT_ENTITY_VERSION],
queryFn: () => GetMetricQueryRange(queryPayload, DEFAULT_ENTITY_VERSION),
});

useEffect(() => {
if (data?.payload?.data?.newResult?.data?.result) {
const currentData = data.payload.data.newResult.data.result;
if (currentData.length > 0 && currentData[0].list) {
if (offset === 0) {
setTraces(currentData[0].list ?? []);
} else {
setTraces((prev) => [...prev, ...(currentData[0].list ?? [])]);
}
}
}
}, [data, offset]);

if (isLoading && traces.length === 0) {
return <Skeleton active />;
}

return (
<div className="host-metric-traces">
<ResizeTable
tableLayout="fixed"
pagination={false}
scroll={{ x: true }}
loading={isLoading && traces.length === 0}
dataSource={traces}
columns={columns}
/>
{isLoading && traces.length > 0 && (
<Skeleton
style={{
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using inline styles. Consider using external stylesheets, CSS classes, or styled components instead. This also applies to the inline styles on lines 166 and 182 in HostMetricsLogs.tsx.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SagarRajput-7 Let me add these changes of Traces in the new PR.

height: '100%',
padding: '16px',
}}
/>
)}
</div>
);
}

export default HostMetricTraces;
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
import { PANEL_TYPES } from 'constants/queryBuilder';
import { GetQueryResultsProps } from 'lib/dashboard/getQueryResults';
import { DataTypes } from 'types/api/queryBuilder/queryAutocompleteResponse';
import { EQueryType } from 'types/common/dashboard';
import { DataSource } from 'types/common/queryBuilder';

import { formatNanoToMS } from './utils';

export const columns = [
{
dataIndex: 'timestamp',
key: 'timestamp',
title: 'Timestamp',
width: 145,
render: (timestamp: string): string => new Date(timestamp).toLocaleString(),
},
{
title: 'Service Name',
dataIndex: ['data', 'serviceName'],
key: 'serviceName-string-tag',
width: 145,
},
{
title: 'Name',
dataIndex: ['data', 'name'],
key: 'name-string-tag',
width: 145,
},
{
title: 'Duration',
dataIndex: ['data', 'durationNano'],
key: 'durationNano-float64-tag',
width: 145,
render: (duration: number): string => `${formatNanoToMS(duration)}`,
},
{
title: 'HTTP Method',
dataIndex: ['data', 'httpMethod'],
key: 'httpMethod-string-tag',
width: 145,
},
{
title: 'Status Code',
dataIndex: ['data', 'responseStatusCode'],
key: 'responseStatusCode-string-tag',
width: 145,
},
];

export const getHostTracesQueryPayload = (
hostName: string,
start: number,
end: number,
offset = 0,
): GetQueryResultsProps => ({
query: {
promql: [],
clickhouse_sql: [],
builder: {
queryData: [
{
dataSource: DataSource.TRACES,
queryName: 'A',
aggregateOperator: 'noop',
aggregateAttribute: {
id: '------false',
Copy link
Contributor

Choose a reason for hiding this comment

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

check if there's an id available - which includes key also, something like 'host.name--string--resource--false'

dataType: DataTypes.EMPTY,
key: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

no key?

isColumn: false,
type: '',
isJSON: false,
},
timeAggregation: 'rate',
spaceAggregation: 'sum',
functions: [],
filters: {
items: [
{
id: 'host-filter',
key: {
key: 'host.name',
dataType: DataTypes.String,
type: 'resource',
isColumn: false,
isJSON: false,
id: 'host.name--string--resource--false',
},
op: '=',
value: hostName,
},
],
op: 'AND',
},
expression: 'A',
disabled: false,
stepInterval: 60,
having: [],
limit: null,
orderBy: [
{
columnName: 'timestamp',
order: 'desc',
},
],
groupBy: [],
legend: '',
reduceTo: 'avg',
},
],
queryFormulas: [],
},
id: '572f1d91-6ac0-46c0-b726-c21488b34434',
rahulkeswani101 marked this conversation as resolved.
Show resolved Hide resolved
queryType: EQueryType.QUERY_BUILDER,
},
graphType: PANEL_TYPES.LIST,
selectedTime: 'GLOBAL_TIME',
params: {
dataSource: DataSource.TRACES,
},
tableParams: {
pagination: {
limit: 10,
offset,
},
selectColumns: [
Copy link
Contributor

Choose a reason for hiding this comment

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

check if we can use the constant - defaultTraceSelectedColumns

{
key: 'serviceName',
dataType: 'string',
type: 'tag',
isColumn: true,
isJSON: false,
id: 'serviceName--string--tag--true',
isIndexed: false,
},
{
key: 'name',
dataType: 'string',
type: 'tag',
isColumn: true,
isJSON: false,
id: 'name--string--tag--true',
isIndexed: false,
},
{
key: 'durationNano',
dataType: 'float64',
type: 'tag',
isColumn: true,
isJSON: false,
id: 'durationNano--float64--tag--true',
isIndexed: false,
},
{
key: 'httpMethod',
dataType: 'string',
type: 'tag',
isColumn: true,
isJSON: false,
id: 'httpMethod--string--tag--true',
isIndexed: false,
},
{
key: 'responseStatusCode',
dataType: 'string',
type: 'tag',
isColumn: true,
isJSON: false,
id: 'responseStatusCode--string--tag--true',
isIndexed: false,
},
],
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const formatNanoToMS = (nanoSeconds: number): string => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The formatNanoToMS function duplicates the conversion logic of nanoToMilli. Consider using nanoToMilli and appending 'ms' to its result to avoid duplication.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

const milliseconds = nanoSeconds / 1_000_000;
return `${milliseconds.toFixed(0)}ms`;
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
background: var(--bg-ink-400);
box-shadow: -4px 10px 16px 2px rgba(0, 0, 0, 0.2);

.ant-drawer-title {
margin-left: .5rem;
}

.ant-drawer-header {
padding: 8px 16px;
border-bottom: none;
Expand Down Expand Up @@ -48,8 +44,6 @@
}

.host-detail-drawer__host {
padding: 2px;

.host-details-grid {
.labels-row,
.values-row {
Expand Down
Loading
Loading