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

Remove highcharts dependency pt. 1 #15970

Merged
merged 23 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
1,884 changes: 753 additions & 1,131 deletions web/vtadmin/package-lock.json

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions web/vtadmin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
"dependencies": {
"@bugsnag/js": "^7.20.0",
"@headlessui/react": "^1.7.8",
"@types/d3": "^7.4.3",
"@types/jest": "^29.4.0",
"@types/node": "^16.11.7",
"@types/react-router-dom": "^5.3.3",
"classnames": "^2.3.2",
"d3": "^7.9.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add @types/d3 too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✨ absolutely, TY

"dayjs": "^1.11.7",
"downshift": "^7.2.0",
"highcharts": "^10.3.3",
"highcharts-react-official": "^3.1.0",
"history": "^5.3.0",
"lodash-es": "^4.17.21",
Expand Down Expand Up @@ -88,7 +89,7 @@
"i": "^0.3.7",
"jsdom": "^21.1.1",
"msw": "^0.36.8",
"npm": "^9.6.3",
"npm": "^10.8.0",
"postcss": "^8.4.31",
"prettier": "^2.2.1",
"protobufjs-cli": "^1.1.1",
Expand Down
145 changes: 145 additions & 0 deletions web/vtadmin/src/components/charts/D3Timeseries.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/**
* Copyright 2024 The Vitess Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as d3 from 'd3';
import { useEffect, useMemo, useRef } from 'react';
import { TimeseriesMap, TimeseriesPoint } from '../../util/tabletDebugVars';

const MARGIN = { top: 30, right: 30, bottom: 50, left: 50 };
const width = 1000;
const height = 500;

type LineChartProps = {
isLoading: boolean;
timeseriesMap: TimeseriesMap;
};

export const D3Timeseries = ({ isLoading, timeseriesMap }: LineChartProps) => {
// bounds = area inside the graph axis = calculated by substracting the margins
const axesRef = useRef(null);
const boundsWidth = width - MARGIN.right - MARGIN.left;
const boundsHeight = height - MARGIN.top - MARGIN.bottom;

const [xRanges, yRanges] = axisMinsAndMaxes(timeseriesMap, boundsWidth);
const yMax = yRanges[1];
const yScale = useMemo(() => {
return d3
.scaleLinear()
.domain([0, yMax || 0])
.range([boundsHeight, 0]);
}, [boundsHeight, yMax]);

const xScale = useMemo(() => {
return d3.scaleTime().domain(xRanges).range([0, boundsWidth]);
}, [boundsWidth, xRanges]);

// Render the X and Y axis using d3.js, not react
useEffect(() => {
const svgElement = d3.select(axesRef.current);
svgElement.selectAll('*').remove();

// Render X Axis
const xAxisGenerator = d3.axisBottom<Date>(xScale);
xAxisGenerator.tickFormat(d3.timeFormat('%H:%M'));
svgElement
.append('g')
.attr('transform', 'translate(0,' + boundsHeight + ')')
.call(xAxisGenerator)
.selectAll('text')
.attr('class', 'fill-gray-500 font-mono text-medium');

// Render Y Axis
const yAxisGenerator = d3.axisLeft(yScale);
svgElement
.append('g')
.call(yAxisGenerator)
.selectAll('text')
.attr('class', 'fill-gray-500 font-mono text-medium');
svgElement.selectAll('path').attr('class', '!stroke-gray-200');
svgElement.selectAll('line').attr('class', '!stroke-gray-200 z-10');
}, [xScale, yScale, boundsHeight]);

// Build the line
const lineBuilder = d3
.line<TimeseriesPoint>()
.x((d) => xScale(d.x))
.y((d) => yScale(d.y));

const colors = d3.scaleOrdinal(d3.schemePiYG[11]);

return (
<div>
<svg width={width} height={height}>
<g
width={boundsWidth}
height={boundsHeight}
transform={`translate(${[MARGIN.left, MARGIN.top].join(',')})`}
>
{Object.entries(timeseriesMap).map(([name, ts], i) => (
<Line color={colors(name)} key={name} timeseriesPoints={ts} lineBuilder={lineBuilder} />
))}
</g>
<g
width={boundsWidth}
height={boundsHeight}
ref={axesRef}
transform={`translate(${[MARGIN.left, MARGIN.top].join(',')})`}
/>
</svg>
<Legend colors={colors} names={Object.keys(timeseriesMap)} />
</div>
);
};

const Legend: React.FC<{ names: string[]; colors: d3.ScaleOrdinal<string, string, never> }> = ({ names, colors }) => {
return (
<div className="width-full flex items-center justify-center">
{names.map((name) => (
<div className="mr-6 font-mono text-sm flex items-center justify-center" key={`${name}_legend`}>
<div className="h-5 w-5 mr-2" style={{ backgroundColor: colors(name) }} />
{name}
</div>
))}
</div>
);
};

type LineProps = {
color: string;
timeseriesPoints: TimeseriesPoint[];
lineBuilder: d3.Line<TimeseriesPoint>;
};

const Line: React.FC<LineProps> = ({ color, timeseriesPoints, lineBuilder }) => {
const linePath = lineBuilder(timeseriesPoints);
if (!linePath) {
return null;
}

return <path d={linePath} opacity={1} stroke={color} fill="none" strokeWidth={2} className="z-100" />;
};

const axisMinsAndMaxes = (timeseriesMap: TimeseriesMap, boundsWidth: number): [[Date, Date], [number, number]] => {
const x_values: number[] = Object.values(timeseriesMap)
.map((points) => points.map((point) => point.x))
.flat();
const y_values = Object.values(timeseriesMap)
.map((points) => points.map((point) => point.y))
.flat();
const y_max = d3.max(y_values) as number;
const x_ranges = d3.extent(x_values.map((x) => new Date(x))) as [Date, Date];
return [x_ranges, [0, y_max || 1]];
};
21 changes: 5 additions & 16 deletions web/vtadmin/src/components/charts/TabletQPSChart.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2021 The Vitess Authors.
* Copyright 2024 The Vitess Authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine, but we don't typically NEED to change the year when updating an existing file.

Copy link
Member

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.

Ah, that's good to know. I thought someone had told me that we needed to in the past (it may have just been for net-new files). I won't update them going forwards.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,13 +14,11 @@
* limitations under the License.
*/

import Highcharts from 'highcharts';
import { useMemo } from 'react';

import { useExperimentalTabletDebugVars } from '../../hooks/api';
import { getQPSTimeseries, QPS_REFETCH_INTERVAL } from '../../util/tabletDebugVars';
import { mergeOptions } from './chartOptions';
import { Timeseries } from './Timeseries';
import { D3Timeseries } from './D3Timeseries';

interface Props {
alias: string;
Expand All @@ -36,17 +34,8 @@ export const TabletQPSChart = ({ alias, clusterID }: Props) => {
}
);

const options = useMemo(() => {
const tsdata = getQPSTimeseries(debugVars?.data, query.dataUpdatedAt);

const series: Highcharts.SeriesOptionsType[] = Object.entries(tsdata).map(([name, data]) => ({
data,
name,
type: 'line',
}));

return mergeOptions({ series });
const tsdata = useMemo(() => {
return getQPSTimeseries(debugVars?.data, query.dataUpdatedAt);
}, [debugVars, query.dataUpdatedAt]);

return <Timeseries isLoading={query.isLoading} options={options} />;
return <D3Timeseries isLoading={query.isLoading} timeseriesMap={tsdata} />;
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2021 The Vitess Authors.
* Copyright 2024 The Vitess Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,13 +14,11 @@
* limitations under the License.
*/

import Highcharts from 'highcharts';
import { useMemo } from 'react';

import { useExperimentalTabletDebugVars } from '../../hooks/api';
import { getVReplicationQPSTimeseries, QPS_REFETCH_INTERVAL } from '../../util/tabletDebugVars';
import { mergeOptions } from './chartOptions';
import { Timeseries } from './Timeseries';
import { D3Timeseries } from './D3Timeseries';

interface Props {
alias: string;
Expand All @@ -36,17 +34,8 @@ export const TabletVReplicationQPSChart = ({ alias, clusterID }: Props) => {
}
);

const options = useMemo(() => {
const tsdata = getVReplicationQPSTimeseries(debugVars?.data, query.dataUpdatedAt);

const series: Highcharts.SeriesOptionsType[] = Object.entries(tsdata).map(([name, data]) => ({
data,
name,
type: 'line',
}));

return mergeOptions({ series });
const tsdata = useMemo(() => {
return getVReplicationQPSTimeseries(debugVars?.data, query.dataUpdatedAt);
}, [debugVars, query.dataUpdatedAt]);

return <Timeseries isLoading={query.isLoading} options={options} />;
return <D3Timeseries isLoading={query.isLoading} timeseriesMap={tsdata} />;
};
73 changes: 0 additions & 73 deletions web/vtadmin/src/components/charts/Timeseries.tsx

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import { UseQueryResult } from 'react-query';
import { TabletDebugVarsResponse } from '../../api/http';
import { vtadmin as pb } from '../../proto/vtadmin';
import { formatSeries } from './WorkflowStreamsLagChart';
import { getWorkflowTimeseries } from './WorkflowStreamsLagChart';

describe('WorkflowStreamsLagChart', () => {
describe('formatSeries', () => {
Expand Down Expand Up @@ -75,23 +75,25 @@ describe('WorkflowStreamsLagChart', () => {

// A sneaky cast to UseQueryResult since otherwise enumerating the many fields
// UseQueryResult (most of which we don't use) is pointlessly verbose.
const result = formatSeries(workflow, queries as UseQueryResult<TabletDebugVarsResponse, Error>[]);
const result = getWorkflowTimeseries(workflow, queries as UseQueryResult<TabletDebugVarsResponse, Error>[]);

console.log(result);
notfelineit marked this conversation as resolved.
Show resolved Hide resolved
// Use snapshot matching since defining expected values for arrays of 180 data points is... annoying.
expect(result).toMatchSnapshot();

// ...but! Add additional validation so that failing tests are easier to debug.
// (And because it can be tempting to not examine snapshot changes in detail...) :)
expect(result.length).toEqual(3);
expect(Object.keys(result).length).toEqual(3);

expect(result[0].name).toEqual('us_east_1a-123456/1');
expect(result[1].name).toEqual('us_east_1a-123456/2');
expect(result[2].name).toEqual('us_east_1a-789012/1');
// expect(result['us_east_1a-123456/1']).toEqual([1, 1, 1])
// expect(result[0].name).toEqual('us_east_1a-123456/1');
// expect(result[1].name).toEqual('us_east_1a-123456/2');
// expect(result[2].name).toEqual('us_east_1a-789012/1');
notfelineit marked this conversation as resolved.
Show resolved Hide resolved
notfelineit marked this conversation as resolved.
Show resolved Hide resolved
});

it('should handle empty input', () => {
const result = formatSeries(null, []);
expect(result).toEqual([]);
const result = getWorkflowTimeseries(null, []);
expect(result).toEqual({});
});
});
});
Loading
Loading