From 1bf8e6bef6645a2460844cb7a1dddfd88d42b15c Mon Sep 17 00:00:00 2001 From: Pranshu Chittora Date: Fri, 11 Mar 2022 11:20:52 +0530 Subject: [PATCH 1/4] feat: better x-axis labels --- frontend/src/components/Graph/index.tsx | 20 +++- frontend/src/components/Graph/xAxisConfig.ts | 113 ++++++++++++++++++ .../GridGraphLayout/Graph/FullView/index.tsx | 1 - .../MetricsApplication/Tabs/Application.tsx | 7 +- frontend/src/container/Trace/Graph/config.ts | 1 - frontend/src/container/Trace/Graph/index.tsx | 10 +- 6 files changed, 139 insertions(+), 13 deletions(-) create mode 100644 frontend/src/components/Graph/xAxisConfig.ts diff --git a/frontend/src/components/Graph/index.tsx b/frontend/src/components/Graph/index.tsx index b6905495d4..a7647dbee7 100644 --- a/frontend/src/components/Graph/index.tsx +++ b/frontend/src/components/Graph/index.tsx @@ -27,6 +27,7 @@ import { useSelector } from 'react-redux'; import { AppState } from 'store/reducers'; import AppReducer from 'types/reducer/app'; +import { ITimeRange, useXAxisTimeUnit } from './xAxisConfig'; Chart.register( LineElement, PointElement, @@ -59,7 +60,11 @@ const Graph = ({ const chartRef = useRef(null); const currentTheme = isDarkMode ? 'dark' : 'light'; - // const [tooltipVisible, setTooltipVisible] = useState(false); + /** + * Computes the relevant time unit for x axis by analyzing the time stamp data + */ + const xAxisTimeUnit = useXAxisTimeUnit(data); + const lineChartRef = useRef(); const getGridColor = useCallback(() => { @@ -109,7 +114,18 @@ const Graph = ({ date: chartjsAdapter, }, time: { - unit: 'minute', + unit: xAxisTimeUnit?.unitName || 'minute', + stepSize: xAxisTimeUnit?.stepSize || 1, + displayFormats: { + millisecond: 'hh:mm:ss', + second: 'hh:mm:ss', + minute: 'HH:mm', + hour: 'MM/dd HH:mm', + day: 'MM/dd', + week: 'MM/dd', + month: 'yy-MM', + year: 'yy', + }, }, type: 'time', }, diff --git a/frontend/src/components/Graph/xAxisConfig.ts b/frontend/src/components/Graph/xAxisConfig.ts new file mode 100644 index 0000000000..a8bc01d05e --- /dev/null +++ b/frontend/src/components/Graph/xAxisConfig.ts @@ -0,0 +1,113 @@ +import { Chart, TimeUnit } from 'chart.js'; +import { useMemo } from 'react'; +import { useSelector } from 'react-redux'; +import { AppState } from 'store/reducers'; +import { GlobalReducer } from 'types/reducer/globalTime'; + +interface IAxisTimeUint { + unitName: TimeUnit; + multiplier: number; +} + +interface IAxisTimeConfig { + unitName: TimeUnit; + stepSize: number; +} + +export interface ITimeRange { + minTime: number | null; + maxTime: number | null; +} + +const TIME_UNITS: IAxisTimeUint[] = [ + { + unitName: 'millisecond', + multiplier: 1, + }, + { + unitName: 'second', + multiplier: 1 / 1e3, + }, + { + unitName: 'minute', + multiplier: 1 / (1e3 * 60), + }, + { + unitName: 'hour', + multiplier: 1 / (1e3 * 60 * 60), + }, + { + unitName: 'day', + multiplier: 1 / (1e3 * 60 * 60 * 24), + }, + { + unitName: 'week', + multiplier: 1 / (1e3 * 60 * 60 * 24 * 7), + }, + { + unitName: 'month', + multiplier: 1 / (1e3 * 60 * 60 * 24 * 30), + }, + { + unitName: 'year', + multiplier: 1 / (1e3 * 60 * 60 * 24 * 365), + }, +]; + +export const useXAxisTimeUnit = (data: Chart['data']): IAxisTimeConfig => { + let localTime: ITimeRange; + { + let minTime = Number.POSITIVE_INFINITY; + let maxTime = Number.NEGATIVE_INFINITY; + data?.labels?.forEach((timeStamp: any) => { + timeStamp = Date.parse(timeStamp); + minTime = Math.min(timeStamp, minTime); + maxTime = Math.max(timeStamp, maxTime); + }); + + localTime = { + minTime: minTime === Number.POSITIVE_INFINITY ? null : minTime, + maxTime: maxTime === Number.NEGATIVE_INFINITY ? null : maxTime, + }; + } + const globalTime = useSelector( + (state) => state.globalTime, + ); + + const { maxTime, minTime } = useMemo(() => { + if (localTime && localTime.maxTime && localTime.minTime) { + return { + minTime: localTime.minTime, + maxTime: localTime.maxTime, + }; + } else { + return { + minTime: globalTime.minTime / 1e6, + maxTime: globalTime.maxTime / 1e6, + }; + } + }, [globalTime, localTime]); + // debugger; + return convertTimeRange(minTime, maxTime); +}; + +const convertTimeRange = (start: number, end: number): IAxisTimeConfig => { + const MIN_INTERVALS = 6; + const range = end - start; + let relevantTimeUnit = TIME_UNITS[1]; + let stepSize = 1; + for (let idx = TIME_UNITS.length - 1; idx >= 0; idx--) { + const timeUnit = TIME_UNITS[idx]; + const units = range * timeUnit.multiplier; + const steps = units / MIN_INTERVALS; + if (steps >= 1) { + relevantTimeUnit = timeUnit; + stepSize = steps; + break; + } + } + return { + unitName: relevantTimeUnit.unitName, + stepSize: Math.floor(stepSize) || 1, + }; +}; diff --git a/frontend/src/container/GridGraphLayout/Graph/FullView/index.tsx b/frontend/src/container/GridGraphLayout/Graph/FullView/index.tsx index 5da66f3394..06665218fc 100644 --- a/frontend/src/container/GridGraphLayout/Graph/FullView/index.tsx +++ b/frontend/src/container/GridGraphLayout/Graph/FullView/index.tsx @@ -82,7 +82,6 @@ const FullView = ({ }; const queryMinMax = getMinMax(selectedTime.enum); - const response = await Promise.all( widget.query .filter((e) => e.query.length !== 0) diff --git a/frontend/src/container/MetricsApplication/Tabs/Application.tsx b/frontend/src/container/MetricsApplication/Tabs/Application.tsx index 005033f999..b96344d263 100644 --- a/frontend/src/container/MetricsApplication/Tabs/Application.tsx +++ b/frontend/src/container/MetricsApplication/Tabs/Application.tsx @@ -1,5 +1,6 @@ import { ActiveElement, Chart, ChartData, ChartEvent } from 'chart.js'; import Graph from 'components/Graph'; +import { getTimeRange, ITimeRange } from 'components/Graph/xAxisConfig'; import { METRICS_PAGE_QUERY_PARAM } from 'constants/query'; import ROUTES from 'constants/routes'; import FullView from 'container/GridGraphLayout/Graph/FullView'; @@ -34,8 +35,7 @@ const Application = ({ getWidget }: DashboardProps): JSX.Element => { urlParams.set(METRICS_PAGE_QUERY_PARAM.endTime, tPlusOne.toString()); history.replace( - `${ - ROUTES.TRACE + `${ROUTES.TRACE }?${urlParams.toString()}&selected={"serviceName":["${servicename}"],"status":["ok","error"]}&filterToFetchData=["duration","status","serviceName"]&userSelectedFilter={"status":["error","ok"],"serviceName":["${servicename}"]}&isSelectedFilterSkipped=true`, ); }; @@ -88,8 +88,7 @@ const Application = ({ getWidget }: DashboardProps): JSX.Element => { urlParams.set(METRICS_PAGE_QUERY_PARAM.endTime, tPlusOne.toString()); history.replace( - `${ - ROUTES.TRACE + `${ROUTES.TRACE }?${urlParams.toString()}&selected={"serviceName":["${servicename}"],"status":["error"]}&filterToFetchData=["duration","status","serviceName"]&userSelectedFilter={"status":["error"],"serviceName":["${servicename}"]}&isSelectedFilterSkipped=true`, ); }; diff --git a/frontend/src/container/Trace/Graph/config.ts b/frontend/src/container/Trace/Graph/config.ts index 17344b52ef..f72e23f97a 100644 --- a/frontend/src/container/Trace/Graph/config.ts +++ b/frontend/src/container/Trace/Graph/config.ts @@ -27,7 +27,6 @@ export const getChartData = ( data: [], type: 'line', }; - const chartLabels: ChartData<'line'>['labels'] = []; Object.keys(allDataPoints).forEach((timestamp) => { diff --git a/frontend/src/container/Trace/Graph/index.tsx b/frontend/src/container/Trace/Graph/index.tsx index efc8a9d137..7ee311cc89 100644 --- a/frontend/src/container/Trace/Graph/index.tsx +++ b/frontend/src/container/Trace/Graph/index.tsx @@ -1,13 +1,13 @@ -import React, { useMemo } from 'react'; - +import { Typography } from 'antd'; import Graph from 'components/Graph'; +import Spinner from 'components/Spinner'; +import React, { useMemo } from 'react'; import { useSelector } from 'react-redux'; import { AppState } from 'store/reducers'; import { TraceReducer } from 'types/reducer/trace'; -import Spinner from 'components/Spinner'; -import { Container } from './styles'; -import { Typography } from 'antd'; + import { getChartData, getChartDataforGroupBy } from './config'; +import { Container } from './styles'; const TraceGraph = () => { const { spansGraph, selectedGroupBy } = useSelector( From f0c627eebe02cca9eb9d1b9e369880495bcb724d Mon Sep 17 00:00:00 2001 From: Pranshu Chittora Date: Fri, 11 Mar 2022 13:39:04 +0530 Subject: [PATCH 2/4] chore: add unit tests --- .../Graph/__tests__/xAxisConfig.test.ts | 75 +++++++++++++++++ frontend/src/components/Graph/index.tsx | 7 +- frontend/src/components/Graph/xAxisConfig.ts | 80 +++++++++++++------ 3 files changed, 134 insertions(+), 28 deletions(-) create mode 100644 frontend/src/components/Graph/__tests__/xAxisConfig.test.ts diff --git a/frontend/src/components/Graph/__tests__/xAxisConfig.test.ts b/frontend/src/components/Graph/__tests__/xAxisConfig.test.ts new file mode 100644 index 0000000000..e0320e29b0 --- /dev/null +++ b/frontend/src/components/Graph/__tests__/xAxisConfig.test.ts @@ -0,0 +1,75 @@ +import { expect } from '@jest/globals'; +import dayjs from 'dayjs'; + +import { convertTimeRange, TIME_UNITS } from '../xAxisConfig'; + +describe('xAxisConfig for Chart', () => { + describe('convertTimeRange', () => { + it('should return relevant time units for given range', () => { + { + const start = dayjs(); + const end = start.add(10, 'millisecond'); + + expect(convertTimeRange(start.valueOf(), end.valueOf()).unitName).toEqual( + TIME_UNITS.millisecond, + ); + } + { + const start = dayjs(); + const end = start.add(10, 'second'); + + expect(convertTimeRange(start.valueOf(), end.valueOf()).unitName).toEqual( + TIME_UNITS.second, + ); + } + { + const start = dayjs(); + const end = start.add(10, 'minute'); + + expect(convertTimeRange(start.valueOf(), end.valueOf()).unitName).toEqual( + TIME_UNITS.minute, + ); + } + { + const start = dayjs(); + const end = start.add(10, 'hour'); + + expect(convertTimeRange(start.valueOf(), end.valueOf()).unitName).toEqual( + TIME_UNITS.hour, + ); + } + { + const start = dayjs(); + const end = start.add(10, 'day'); + + expect(convertTimeRange(start.valueOf(), end.valueOf()).unitName).toEqual( + TIME_UNITS.day, + ); + } + { + const start = dayjs(); + const end = start.add(10, 'week'); + + expect(convertTimeRange(start.valueOf(), end.valueOf()).unitName).toEqual( + TIME_UNITS.week, + ); + } + { + const start = dayjs(); + const end = start.add(10, 'month'); + + expect(convertTimeRange(start.valueOf(), end.valueOf()).unitName).toEqual( + TIME_UNITS.month, + ); + } + { + const start = dayjs(); + const end = start.add(10, 'year'); + + expect(convertTimeRange(start.valueOf(), end.valueOf()).unitName).toEqual( + TIME_UNITS.year, + ); + } + }); + }); +}); diff --git a/frontend/src/components/Graph/index.tsx b/frontend/src/components/Graph/index.tsx index a7647dbee7..4cf6c04a0f 100644 --- a/frontend/src/components/Graph/index.tsx +++ b/frontend/src/components/Graph/index.tsx @@ -27,7 +27,7 @@ import { useSelector } from 'react-redux'; import { AppState } from 'store/reducers'; import AppReducer from 'types/reducer/app'; -import { ITimeRange, useXAxisTimeUnit } from './xAxisConfig'; +import { useXAxisTimeUnit } from './xAxisConfig'; Chart.register( LineElement, PointElement, @@ -60,10 +60,7 @@ const Graph = ({ const chartRef = useRef(null); const currentTheme = isDarkMode ? 'dark' : 'light'; - /** - * Computes the relevant time unit for x axis by analyzing the time stamp data - */ - const xAxisTimeUnit = useXAxisTimeUnit(data); + const xAxisTimeUnit = useXAxisTimeUnit(data); // Computes the relevant time unit for x axis by analyzing the time stamp data const lineChartRef = useRef(); diff --git a/frontend/src/components/Graph/xAxisConfig.ts b/frontend/src/components/Graph/xAxisConfig.ts index a8bc01d05e..493c36e5b7 100644 --- a/frontend/src/components/Graph/xAxisConfig.ts +++ b/frontend/src/components/Graph/xAxisConfig.ts @@ -4,7 +4,10 @@ import { useSelector } from 'react-redux'; import { AppState } from 'store/reducers'; import { GlobalReducer } from 'types/reducer/globalTime'; -interface IAxisTimeUint { +interface ITimeUnit { + [key: string]: TimeUnit; +} +interface IAxisTimeUintConfig { unitName: TimeUnit; multiplier: number; } @@ -19,44 +22,59 @@ export interface ITimeRange { maxTime: number | null; } -const TIME_UNITS: IAxisTimeUint[] = [ +export const TIME_UNITS: ITimeUnit = { + millisecond: 'millisecond', + second: 'second', + minute: 'minute', + hour: 'hour', + day: 'day', + week: 'week', + month: 'month', + year: 'year', +}; + +const TIME_UNITS_CONFIG: IAxisTimeUintConfig[] = [ { - unitName: 'millisecond', + unitName: TIME_UNITS.millisecond, multiplier: 1, }, { - unitName: 'second', + unitName: TIME_UNITS.second, multiplier: 1 / 1e3, }, { - unitName: 'minute', + unitName: TIME_UNITS.minute, multiplier: 1 / (1e3 * 60), }, { - unitName: 'hour', + unitName: TIME_UNITS.hour, multiplier: 1 / (1e3 * 60 * 60), }, { - unitName: 'day', + unitName: TIME_UNITS.day, multiplier: 1 / (1e3 * 60 * 60 * 24), }, { - unitName: 'week', + unitName: TIME_UNITS.week, multiplier: 1 / (1e3 * 60 * 60 * 24 * 7), }, { - unitName: 'month', + unitName: TIME_UNITS.month, multiplier: 1 / (1e3 * 60 * 60 * 24 * 30), }, { - unitName: 'year', + unitName: TIME_UNITS.year, multiplier: 1 / (1e3 * 60 * 60 * 24 * 365), }, ]; +/** + * Accepts Chart.js data's data-structure and returns the relevant time unit for the axis based on the range of the data. + */ export const useXAxisTimeUnit = (data: Chart['data']): IAxisTimeConfig => { - let localTime: ITimeRange; - { + // Local time is the time range inferred from the input chart data. + let localTime: ITimeRange | null; + try { let minTime = Number.POSITIVE_INFINITY; let maxTime = Number.NEGATIVE_INFINITY; data?.labels?.forEach((timeStamp: any) => { @@ -69,11 +87,17 @@ export const useXAxisTimeUnit = (data: Chart['data']): IAxisTimeConfig => { minTime: minTime === Number.POSITIVE_INFINITY ? null : minTime, maxTime: maxTime === Number.NEGATIVE_INFINITY ? null : maxTime, }; + } catch (error) { + localTime = null; + console.error(error); } + + // Global time is the time selected from the global time selector menu. const globalTime = useSelector( (state) => state.globalTime, ); + // Use local time if valid else use the global time range const { maxTime, minTime } = useMemo(() => { if (localTime && localTime.maxTime && localTime.minTime) { return { @@ -87,24 +111,34 @@ export const useXAxisTimeUnit = (data: Chart['data']): IAxisTimeConfig => { }; } }, [globalTime, localTime]); - // debugger; + return convertTimeRange(minTime, maxTime); }; -const convertTimeRange = (start: number, end: number): IAxisTimeConfig => { +/** + * Finds the relevant time unit based on the input time stamps (in ms) + */ +export const convertTimeRange = ( + start: number, + end: number, +): IAxisTimeConfig => { const MIN_INTERVALS = 6; const range = end - start; - let relevantTimeUnit = TIME_UNITS[1]; + let relevantTimeUnit = TIME_UNITS_CONFIG[1]; let stepSize = 1; - for (let idx = TIME_UNITS.length - 1; idx >= 0; idx--) { - const timeUnit = TIME_UNITS[idx]; - const units = range * timeUnit.multiplier; - const steps = units / MIN_INTERVALS; - if (steps >= 1) { - relevantTimeUnit = timeUnit; - stepSize = steps; - break; + try { + for (let idx = TIME_UNITS_CONFIG.length - 1; idx >= 0; idx--) { + const timeUnit = TIME_UNITS_CONFIG[idx]; + const units = range * timeUnit.multiplier; + const steps = units / MIN_INTERVALS; + if (steps >= 1) { + relevantTimeUnit = timeUnit; + stepSize = steps; + break; + } } + } catch (error) { + console.error(error); } return { unitName: relevantTimeUnit.unitName, From f2ace729fdeb96adbdae409054ef310bcb55efc9 Mon Sep 17 00:00:00 2001 From: Pranshu Chittora Date: Fri, 11 Mar 2022 13:42:05 +0530 Subject: [PATCH 3/4] fix: linting fixes --- .../src/container/MetricsApplication/Tabs/Application.tsx | 7 ++++--- frontend/src/container/Trace/Graph/index.tsx | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/frontend/src/container/MetricsApplication/Tabs/Application.tsx b/frontend/src/container/MetricsApplication/Tabs/Application.tsx index b96344d263..005033f999 100644 --- a/frontend/src/container/MetricsApplication/Tabs/Application.tsx +++ b/frontend/src/container/MetricsApplication/Tabs/Application.tsx @@ -1,6 +1,5 @@ import { ActiveElement, Chart, ChartData, ChartEvent } from 'chart.js'; import Graph from 'components/Graph'; -import { getTimeRange, ITimeRange } from 'components/Graph/xAxisConfig'; import { METRICS_PAGE_QUERY_PARAM } from 'constants/query'; import ROUTES from 'constants/routes'; import FullView from 'container/GridGraphLayout/Graph/FullView'; @@ -35,7 +34,8 @@ const Application = ({ getWidget }: DashboardProps): JSX.Element => { urlParams.set(METRICS_PAGE_QUERY_PARAM.endTime, tPlusOne.toString()); history.replace( - `${ROUTES.TRACE + `${ + ROUTES.TRACE }?${urlParams.toString()}&selected={"serviceName":["${servicename}"],"status":["ok","error"]}&filterToFetchData=["duration","status","serviceName"]&userSelectedFilter={"status":["error","ok"],"serviceName":["${servicename}"]}&isSelectedFilterSkipped=true`, ); }; @@ -88,7 +88,8 @@ const Application = ({ getWidget }: DashboardProps): JSX.Element => { urlParams.set(METRICS_PAGE_QUERY_PARAM.endTime, tPlusOne.toString()); history.replace( - `${ROUTES.TRACE + `${ + ROUTES.TRACE }?${urlParams.toString()}&selected={"serviceName":["${servicename}"],"status":["error"]}&filterToFetchData=["duration","status","serviceName"]&userSelectedFilter={"status":["error"],"serviceName":["${servicename}"]}&isSelectedFilterSkipped=true`, ); }; diff --git a/frontend/src/container/Trace/Graph/index.tsx b/frontend/src/container/Trace/Graph/index.tsx index 7ee311cc89..ce667aec67 100644 --- a/frontend/src/container/Trace/Graph/index.tsx +++ b/frontend/src/container/Trace/Graph/index.tsx @@ -9,7 +9,7 @@ import { TraceReducer } from 'types/reducer/trace'; import { getChartData, getChartDataforGroupBy } from './config'; import { Container } from './styles'; -const TraceGraph = () => { +const TraceGraph = (): JSX.Element => { const { spansGraph, selectedGroupBy } = useSelector( (state) => state.traces, ); From a95656b3a0e3d665d0f214d2e6251bf9e055ebfe Mon Sep 17 00:00:00 2001 From: Pranshu Chittora Date: Wed, 16 Mar 2022 15:29:23 +0530 Subject: [PATCH 4/4] fix: x axis label when the time stamp is not parsed --- frontend/src/components/Graph/xAxisConfig.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/Graph/xAxisConfig.ts b/frontend/src/components/Graph/xAxisConfig.ts index 493c36e5b7..ae99a49a23 100644 --- a/frontend/src/components/Graph/xAxisConfig.ts +++ b/frontend/src/components/Graph/xAxisConfig.ts @@ -77,8 +77,8 @@ export const useXAxisTimeUnit = (data: Chart['data']): IAxisTimeConfig => { try { let minTime = Number.POSITIVE_INFINITY; let maxTime = Number.NEGATIVE_INFINITY; - data?.labels?.forEach((timeStamp: any) => { - timeStamp = Date.parse(timeStamp); + data?.labels?.forEach((timeStamp: string | number): void => { + if (typeof timeStamp === 'string') timeStamp = Date.parse(timeStamp); minTime = Math.min(timeStamp, minTime); maxTime = Math.max(timeStamp, maxTime); });