Skip to content

Commit

Permalink
ui: make Metrics and SQL timepicker align
Browse files Browse the repository at this point in the history
Previously, the timepicker from Metrics page and
the timepicker on SQL Activity pages acted independently.
Now, if the value of one changes, the other value changes
to the same period selected.

This commit also fixes a bug where the period selected
would change to a custom value if the Metrics page was
refreshed.

Fixes #78187
Fixes #82152

Release note (ui change): The period selected on the Metrics
page and the SQL Activity pages are now aligned. If the user
changes in one page, the value will be the same for the other.

Release note (bug fix): The period selected on Metrics time picker
continues the same when refreshing the page, no longer changing
to a custom period.
  • Loading branch information
maryliag committed Jun 24, 2022
1 parent 558370b commit 78b723d
Show file tree
Hide file tree
Showing 17 changed files with 96 additions and 76 deletions.
15 changes: 15 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,26 @@ import moment from "moment";
import {
defaultTimeScaleOptions,
findClosestTimeScale,
toDateRange,
toRoundedDateRange,
} from "./utils";
import { assert } from "chai";

describe("timescale utils", (): void => {
describe("toDateRange", () => {
it("get date range", () => {
const ts: TimeScale = {
windowSize: moment.duration(5, "day"),
sampleSize: moment.duration(5, "minutes"),
fixedWindowEnd: moment.utc("2022.01.10 13:42"),
key: "Custom",
};
const [start, end] = toDateRange(ts);
assert.equal(start.format("YYYY.MM.DD HH:mm:ss"), "2022.01.05 13:42:00");
assert.equal(end.format("YYYY.MM.DD HH:mm:ss"), "2022.01.10 13:42:00");
});
});

describe("toRoundedDateRange", () => {
it("round values", () => {
const ts: TimeScale = {
Expand Down
9 changes: 7 additions & 2 deletions pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,17 @@ export const defaultTimeScaleSelected: TimeScale = {
fixedWindowEnd: false,
};

// toDateRange returns the actual value of start and end date, based on
// the timescale.
// Since this value is used on componentDidUpdate, we don't want a refresh
// to happen every millisecond, so we set the millisecond value to 0.
export const toDateRange = (ts: TimeScale): [moment.Moment, moment.Moment] => {
const end = ts.fixedWindowEnd
? moment.utc(ts.fixedWindowEnd)
: moment().utc();
const start = moment.utc(end).subtract(ts.windowSize);
return [start, end];
const endRounded = end.set({ millisecond: 0 });
const start = moment.utc(endRounded).subtract(ts.windowSize);
return [start, endRounded];
};

// toRoundedDateRange round the TimeScale selected, with the start
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { TimeScale, defaultTimeScaleSelected } from "@cockroachlabs/cluster-ui";

const localSettingsSelector = (state: AdminUIState) => state.localSettings;

export const statementsTimeScaleLocalSetting = new LocalSetting<
export const globalTimeScaleLocalSetting = new LocalSetting<
AdminUIState,
TimeScale
>("timeScale/SQLActivity", localSettingsSelector, defaultTimeScaleSelected);
2 changes: 1 addition & 1 deletion pkg/ui/workspaces/db-console/src/redux/sqlActivity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// licenses/APL.txt.
import { Action } from "redux";
import _ from "lodash";
import { PayloadAction } from "oss/src/interfaces/action";
import { PayloadAction } from "src/interfaces/action";

/**
* SqlActivityState maintains a MetricQuerySet collection, along with some
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ export function createOpenDiagnosticsModalAction(
Combined Stats Actions
****************************************/

export const SET_COMBINED_STATEMENTS_TIME_SCALE =
"cockroachui/statements/SET_COMBINED_STATEMENTS_TIME_SCALE";
export const SET_GLOBAL_TIME_SCALE =
"cockroachui/statements/SET_GLOBAL_TIME_SCALE";

export function setCombinedStatementsTimeScaleAction(
export function setGlobalTimeScaleAction(
ts: TimeScale,
): PayloadAction<TimeScale> {
return {
type: SET_COMBINED_STATEMENTS_TIME_SCALE,
type: SET_GLOBAL_TIME_SCALE,
payload: ts,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
CreateStatementDiagnosticsReportPayload,
createStatementDiagnosticsReportCompleteAction,
createStatementDiagnosticsReportFailedAction,
SET_COMBINED_STATEMENTS_TIME_SCALE,
SET_GLOBAL_TIME_SCALE,
CANCEL_STATEMENT_DIAGNOSTICS_REPORT,
cancelStatementDiagnosticsReportCompleteAction,
cancelStatementDiagnosticsReportFailedAction,
Expand All @@ -41,7 +41,7 @@ import {
createStatementDiagnosticsAlertLocalSetting,
cancelStatementDiagnosticsAlertLocalSetting,
} from "src/redux/alerts";
import { statementsTimeScaleLocalSetting } from "src/redux/statementsTimeScale";
import { globalTimeScaleLocalSetting } from "src/redux/globalTimeScale";
import { TimeScale, toDateRange } from "@cockroachlabs/cluster-ui";
import Long from "long";

Expand Down Expand Up @@ -156,7 +156,7 @@ export function* setCombinedStatementsTimeScaleSaga(
) {
const ts = action.payload;

yield put(statementsTimeScaleLocalSetting.set(ts));
yield put(globalTimeScaleLocalSetting.set(ts));
const [start, end] = toDateRange(ts);
const req = new CombinedStatementsRequest({
combined: true,
Expand All @@ -171,9 +171,6 @@ export function* statementsSaga() {
yield all([
takeEvery(CREATE_STATEMENT_DIAGNOSTICS_REPORT, createDiagnosticsReportSaga),
takeEvery(CANCEL_STATEMENT_DIAGNOSTICS_REPORT, cancelDiagnosticsReportSaga),
takeLatest(
SET_COMBINED_STATEMENTS_TIME_SCALE,
setCombinedStatementsTimeScaleSaga,
),
takeLatest(SET_GLOBAL_TIME_SCALE, setCombinedStatementsTimeScaleSaga),
]);
}
6 changes: 1 addition & 5 deletions pkg/ui/workspaces/db-console/src/redux/timeScale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,7 @@ export function timeScaleReducer(
case SET_SCALE: {
const { payload: scale } = action as PayloadAction<TimeScale>;
state = _.cloneDeep(state);
if (scale.key === "Custom") {
state.metricsTime.isFixedWindow = true;
} else {
state.metricsTime.isFixedWindow = false;
}
state.metricsTime.isFixedWindow = scale.key === "Custom";
state.scale = scale;
state.metricsTime.shouldUpdateMetricsWindowFromScale = true;
return state;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ class MetricsTimeManager extends React.Component<

// Fixed time ranges can't expire.
if (props.timeScale.scale.fixedWindowEnd) {
// this.setWindow(props);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import {
Metric,
MetricProps,
MetricsDataComponentProps,
QueryTimeInfo,
} from "src/views/shared/components/metricQuery";
import {} from "@cockroachlabs/cluster-ui";
import {
calculateXAxisDomain,
calculateYAxisDomain,
Expand Down Expand Up @@ -193,8 +193,7 @@ export class LineGraph extends React.Component<LineGraphProps, {}> {
},
);

// setNewTimeRange uses code from the TimeScaleDropdown component
// to set new start/end ranges in the query params and force a
// setNewTimeRange forces a
// reload of the rest of the dashboard at new ranges via the props
// `setMetricsFixedWindow` and `setTimeScale`.
// TODO(davidh): centralize management of query params for time range
Expand Down Expand Up @@ -224,9 +223,6 @@ export class LineGraph extends React.Component<LineGraphProps, {}> {
const { pathname, search } = this.props.history.location;
const urlParams = new URLSearchParams(search);

urlParams.set("start", moment.unix(start).format("X"));
urlParams.set("end", moment.unix(end).format("X"));

this.props.history.push({
pathname,
search: urlParams.toString(),
Expand All @@ -252,10 +248,29 @@ export class LineGraph extends React.Component<LineGraphProps, {}> {
// to a closure that holds a reference to this value.
xAxisDomain: AxisDomain;

newTimeInfo(
newTimeInfo: QueryTimeInfo,
prevTimeInfo: QueryTimeInfo,
): boolean {
if (newTimeInfo.start.compare(prevTimeInfo.start) !== 0) {
return true;
}
if (newTimeInfo.end.compare(prevTimeInfo.end) !== 0) {
return true;
}
if (newTimeInfo.sampleDuration.compare(prevTimeInfo.sampleDuration) !== 0) {
return true;
}

return false;
}

componentDidUpdate(prevProps: Readonly<LineGraphProps>) {
if (
!this.props.data?.results ||
(prevProps.data === this.props.data && this.u !== undefined)
(prevProps.data === this.props.data &&
this.u !== undefined &&
!this.newTimeInfo(this.props.timeInfo, prevProps.timeInfo))
) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import { configureUPlotLineChart } from "src/views/cluster/util/graphs";
import Long from "long";

describe("<LineGraph>", function () {
let spy: sinon.SinonSpy;
let mockProps: LineGraphProps;
const linegraph = (props: LineGraphProps) =>
shallow(
Expand Down Expand Up @@ -73,7 +72,6 @@ describe("<LineGraph>", function () {
},
},
};
spy = sinon.spy();
});

it("should render a root component on mount", () => {
Expand All @@ -82,18 +80,6 @@ describe("<LineGraph>", function () {
assert.equal(root.length, 1);
});

it("should set new history", () => {
const wrapper = linegraph({
...mockProps,
history: { ...mockProps.history, push: spy },
});
const instance = wrapper.instance() as any as LineGraph;
instance.setNewTimeRange(111111, 222222);
assert.isTrue(
spy.calledWith({ pathname: "", search: "start=111&end=222" }),
);
});

it("should set a new chart on update", () => {
const wrapper = linegraph({ ...mockProps });
const instance = wrapper.instance() as any as LineGraph;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
PageConfig,
PageConfigItem,
} from "src/views/shared/components/pageconfig";
import TimeScaleDropdown from "src/views/cluster/containers/timeScaleDropdownWithSearchParams";
import ClusterSummaryBar from "./summaryBar";

import { AdminUIState } from "src/redux/state";
Expand Down Expand Up @@ -66,19 +65,20 @@ import { getMatchParamByName } from "src/util/query";
import { PayloadAction } from "src/interfaces/action";
import {
setMetricsFixedWindow,
setTimeScale,
TimeWindow,
TimeScale,
adjustTimeScale,
} from "src/redux/timeScale";
import { InlineAlert } from "src/components";
import { Anchor } from "@cockroachlabs/cluster-ui";
import { Anchor, TimeScaleDropdown } from "@cockroachlabs/cluster-ui";
import { reduceStorageOfTimeSeriesDataOperationalFlags } from "src/util/docs";
import moment from "moment";
import {
selectResolution10sStorageTTL,
selectResolution30mStorageTTL,
} from "src/redux/clusterSettings";
import { setGlobalTimeScaleAction } from "src/redux/statements";
import { globalTimeScaleLocalSetting } from "src/redux/globalTimeScale";
interface GraphDashboard {
label: string;
component: (props: GraphDashboardProps) => React.ReactElement<any>[];
Expand Down Expand Up @@ -113,6 +113,7 @@ type MapStateToProps = {
hoverState: HoverState;
resolution10sStorageTTL: moment.Duration;
resolution30mStorageTTL: moment.Duration;
timeScale: TimeScale;
};

type MapDispatchToProps = {
Expand Down Expand Up @@ -350,6 +351,8 @@ export class NodeGraphs extends React.Component<
</PageConfigItem>
<PageConfigItem>
<TimeScaleDropdown
currentScale={this.props.timeScale}
setTimeScale={this.props.setTimeScale}
adjustTimeScaleOnChange={this.adjustTimeScaleOnChange}
/>
</PageConfigItem>
Expand Down Expand Up @@ -420,6 +423,7 @@ const mapStateToProps = (state: AdminUIState): MapStateToProps => ({
hoverState: hoverStateSelector(state),
resolution10sStorageTTL: selectResolution10sStorageTTL(state),
resolution30mStorageTTL: selectResolution30mStorageTTL(state),
timeScale: globalTimeScaleLocalSetting.selector(state),
});

const mapDispatchToProps: MapDispatchToProps = {
Expand All @@ -429,7 +433,7 @@ const mapDispatchToProps: MapDispatchToProps = {
hoverOn,
hoverOff,
setMetricsFixedWindow: setMetricsFixedWindow,
setTimeScale: setTimeScale,
setTimeScale: setGlobalTimeScaleAction,
};

export default compose(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ import { refreshMetricMetadata, refreshNodes } from "src/redux/apiReducers";
import { nodesSummarySelector, NodesSummary } from "src/redux/nodes";
import { AdminUIState } from "src/redux/state";
import { LineGraph } from "src/views/cluster/components/linegraph";
import TimeScaleDropdown from "src/views/cluster/containers/timeScaleDropdownWithSearchParams";
import { DropdownOption } from "src/views/shared/components/dropdown";
import { MetricsDataProvider } from "src/views/shared/containers/metricDataProvider";
import { Metric, Axis } from "src/views/shared/components/metricQuery";
import { AxisUnits } from "@cockroachlabs/cluster-ui";
import { AxisUnits, TimeScaleDropdown } from "@cockroachlabs/cluster-ui";
import {
PageConfig,
PageConfigItem,
Expand All @@ -42,8 +41,9 @@ import {
TimeWindow,
TimeScale,
setMetricsFixedWindow,
setTimeScale,
} from "src/redux/timeScale";
import { setGlobalTimeScaleAction } from "src/redux/statements";
import { globalTimeScaleLocalSetting } from "src/redux/globalTimeScale";

export interface CustomChartProps {
refreshNodes: typeof refreshNodes;
Expand All @@ -52,6 +52,7 @@ export interface CustomChartProps {
refreshMetricMetadata: typeof refreshMetricMetadata;
metricsMetadata: MetricsMetadata;
setMetricsFixedWindow: (tw: TimeWindow) => PayloadAction<TimeWindow>;
timeScale: TimeScale;
setTimeScale: (ts: TimeScale) => PayloadAction<TimeScale>;
}

Expand Down Expand Up @@ -293,7 +294,10 @@ export class CustomChart extends React.Component<
</section>
<PageConfig>
<PageConfigItem>
<TimeScaleDropdown />
<TimeScaleDropdown
currentScale={this.props.timeScale}
setTimeScale={this.props.setTimeScale}
/>
</PageConfigItem>
<button
className="edit-button chart-edit-button chart-edit-button--add"
Expand All @@ -307,7 +311,7 @@ export class CustomChart extends React.Component<
<div className="chart-group l-columns__left">
{this.renderCharts()}
</div>
<div className="l-columns__right"></div>
<div className="l-columns__right" />
</div>
</section>
<section className="section">{this.renderChartTables()}</section>
Expand All @@ -320,13 +324,14 @@ const mapStateToProps = (state: AdminUIState) => ({
nodesSummary: nodesSummarySelector(state),
nodesQueryValid: state.cachedData.nodes.valid,
metricsMetadata: metricsMetadataSelector(state),
timeScale: globalTimeScaleLocalSetting.selector(state),
});

const mapDispatchToProps = {
refreshNodes,
refreshMetricMetadata,
setMetricsFixedWindow: setMetricsFixedWindow,
setTimeScale,
setTimeScale: setGlobalTimeScaleAction,
};

export default withRouter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
} from "@cockroachlabs/cluster-ui";
import { History } from "history";
import { refreshSettings } from "src/redux/apiReducers";
import { globalTimeScaleLocalSetting } from "src/redux/globalTimeScale";

/**
* queryFromProps is a helper method which generates a TimeSeries Query data
Expand Down Expand Up @@ -248,13 +249,10 @@ class MetricsDataProvider extends React.Component<
// timeInfoSelector converts the current global time window into a set of Long
// timestamps, which can be sent with requests to the server.
const timeInfoSelector = createSelector(
(state: AdminUIState) => state.timeScale,
tw => {
if (!_.isObject(tw.scale)) {
return null;
}

const [startMoment, endMoment] = toDateRange(tw.scale);
(state: AdminUIState) => state,
state => {
const scale = globalTimeScaleLocalSetting.selector(state);
const [startMoment, endMoment] = toDateRange(scale);
const start = startMoment.valueOf();
const end = endMoment.valueOf();
const syncedScale = findClosestTimeScale(
Expand Down
Loading

0 comments on commit 78b723d

Please sign in to comment.