Skip to content

Commit

Permalink
ui: Rename variables and add comments
Browse files Browse the repository at this point in the history
Finishes addressing cockroachdb#75579

This commit adds comments and renames various variables to better reflect their
meaning and usage. https://github.com/cockroachlabs/managed-service/pull/7998
also renames `windowEnd` -> `fixedWindowEnd` in the managed-service repo.

The following renames were made:
- `scaleChanged` -> `shouldUpdateMetricsWindowFromScale`
- `useTimeRange` -> `isFixedWindow`
- `SET_WINDOW` -> `SET_METRICS_MOVING_WINDOW`, `setTimeWindow` ->
  `setMetricsMovingWindow`
- `SET_RANGE` -> `SET_METRICS_FIXED_WINDOW`, `setTimeRange` ->
  `setMetricsFixedWindow`
- `windowEnd` -> `fixedWindowEnd`, with its type changed from `moment.Moment |
  null` to `moment.Moment | false`. `false` indicates that the end time is a
  dynamically moving "now"
- `db-console/src/redux/timewindow.ts` -> `db-console/src/redux/timeScale.ts `,
  same for the sibling test file, `TimeWindowState` -> `TimeScaleState`,
  `timeWindowReducer` -> `timeScaleReducer`. To make it clear that the `scale`
  property is the state to use, and that `TimeWindow` is mostly just a type
  interface (except for the db console metrics page polling usage)
- `TimeWindowManager` -> `MetricsTimeManager`,
  `db-console/src/views/app/containers/timewindow/index.tsx` ->
  `db-console/src/views/app/containers/metricsTimeManager/index.tsx`, and the
  sibling test file. To distinguish from the `TimeWindow` type interface, name
  it for the main manager component in the file, and make it clear that this is
  just for the metrics page
- `db-console/src/views/cluster/containers/timescale/index.tsx` ->
  `db-console/src/views/cluster/containers/timeScaleDropdownWithSearchParams/index.tsx`,
  and the sibling `.styl` file

Release note: None
  • Loading branch information
jocrl authored and RajivTS committed Mar 6, 2022
1 parent f2f813a commit 9fa03c7
Show file tree
Hide file tree
Showing 26 changed files with 284 additions and 207 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export const getStatementDetailsPropsFixture = (): StatementDetailsProps => ({
timeScale: {
windowSize: moment.duration(5, "day"),
sampleSize: moment.duration(5, "minutes"),
windowEnd: moment.utc("2021.12.12"),
fixedWindowEnd: moment.utc("2021.12.12"),
key: "Custom",
},
statement: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
timeScale: {
windowSize: moment.duration(5, "day"),
sampleSize: moment.duration(5, "minutes"),
windowEnd: moment.utc("2021.12.12"),
fixedWindowEnd: moment.utc("2021.12.12"),
key: "Custom"
},
apps: ["$ internal", "movr", "$ cockroach demo"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
setTimeScale,
adjustTimeScaleOnChange,
}): React.ReactElement => {
const end = currentScale.windowEnd
? moment.utc(currentScale.windowEnd)
const end = currentScale.fixedWindowEnd
? moment.utc(currentScale.fixedWindowEnd)
: moment().utc();
const currentWindow: TimeWindow = {
start: moment.utc(end).subtract(currentScale.windowSize),
Expand All @@ -136,7 +136,7 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
let timeScale: TimeScale = {
...options[rangeOption.label],
key: rangeOption.label,
windowEnd: null,
fixedWindowEnd: false,
};
if (adjustTimeScaleOnChange) {
const timeWindow: TimeWindow = {
Expand All @@ -156,37 +156,43 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
const seconds = windowSize.asSeconds();
let selected = {};
let key = currentScale.key;
let windowEnd = moment.utc(currentWindow.end);
let endTime = moment.utc(currentWindow.end);

switch (direction) {
case ArrowDirection.RIGHT:
if (windowEnd) {
windowEnd = windowEnd.add(seconds, "seconds");
}
endTime = endTime.add(seconds, "seconds");
break;
case ArrowDirection.LEFT:
windowEnd = windowEnd.subtract(seconds, "seconds");
endTime = endTime.subtract(seconds, "seconds");
break;
case ArrowDirection.CENTER:
// CENTER is used to set the time window to the current time.
windowEnd = moment.utc();
endTime = moment.utc();
break;
default:
console.error("Unknown direction: ", direction);
}

// If the timescale extends into the future then fallback to a default
// timescale. Otherwise set the key to "Custom" so it appears correctly.
if (
!windowEnd ||
windowEnd > moment.utc().subtract(currentScale.windowValid)
) {
// The first `!endTime` part of the if clause seems unnecessary since endTime is always a specific time.
// If endTime + windowValid > now. Unclear why this uses windowValid instead of windowSize.
if (!endTime || endTime > moment.utc().subtract(currentScale.windowValid)) {
const foundTimeScale = Object.entries(options).find(
// eslint-disable-next-line @typescript-eslint/no-unused-vars
([_, value]) => value.windowSize.asSeconds() === windowSize.asSeconds(),
);
if (foundTimeScale) {
/**
* This code can be hit by:
* - Select a default option, then click the left arrow, then click the right arrow.
* This (or the parent if block) is *not* hit by:
* - Select a default time, click left, select a custom time of the same range, then click right. The arrow is
* not disabled, but the clause doesn't seem to be true.
*/
selected = { key: foundTimeScale[0], ...foundTimeScale[1] };
} else {
// This code might not be possible to hit, due to the right arrow being disabled
key = "Custom";
}
} else {
Expand All @@ -195,7 +201,7 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({

let timeScale: TimeScale = {
...currentScale,
windowEnd,
fixedWindowEnd: endTime,
windowSize,
key,
...selected,
Expand Down Expand Up @@ -225,7 +231,7 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
let timeScale: TimeScale = {
...findClosestTimeScale(options, seconds),
windowSize: moment.duration(end.diff(start)),
windowEnd: end,
fixedWindowEnd: end,
key: "Custom",
};
if (adjustTimeScaleOnChange) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,42 @@ export interface TimeWindow {
}

/**
* TimeScale describes the requested dimensions of TimeWindows; it
* prescribes a length for the window, along with a period of time that a
* newly created TimeWindow will remain valid.
* TimeScale describes the requested dimensions, from which one can derive concrete TimeWindows using toDateRange.
*/
export interface TimeScale {
// The key used to index in to the availableTimeScales collection.
/**
* The key used to index in to the defaultTimeScaleOptions collection.
* The key is "Custom" when specifying a custom time that is not one of the default options
*/
key?: string;
// The size of a global time window. Default is ten minutes.
windowSize: moment.Duration;
// The length of time the global time window is valid. The current time window
// is invalid if now > (currentWindow.end + windowValid). Default is ten
// seconds. If windowEnd is set this is ignored.
// The length of time the global time window is valid. Default is ten seconds.
windowValid?: moment.Duration;
// The expected duration of individual samples for queries at this time scale.
sampleSize: moment.Duration;
// The end time of the window, or null if it should be a dynamically moving "now".
windowEnd: moment.Moment | null;
/**
* The fixed end time of the window, or false if it should be a dynamically moving "now".
* Typically, when the `key` property is a default option, `fixedWindowEnd` should be false.
* And when the `key` property is "Custom" `fixedWindowEnd` should be a specific Moment.
* It is unclear if there are legitimate reasons for the two being out of sync.
*/
fixedWindowEnd: moment.Moment | false;
}

export class TimeWindowState {
export class TimeScaleState {
// Currently selected scale.
scale: TimeScale;
constructor() {
this.scale = {
...defaultTimeScaleOptions["Past 10 Minutes"],
windowEnd: null,
fixedWindowEnd: false,
key: "Past 10 Minutes",
};
}
}

export type TimeScaleOption = Omit<TimeScale, "windowEnd">;
export type TimeScaleOption = Omit<TimeScale, "fixedWindowEnd">;

export interface TimeScaleOptions {
[key: string]: TimeScaleOption;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
TimeScaleDropdown,
} from "./timeScaleDropdown";
import { defaultTimeScaleOptions, findClosestTimeScale } from "./utils";
import * as timewindow from "./timeScaleTypes";
import * as timescale from "./timeScaleTypes";
import moment from "moment";
import { MemoryRouter } from "react-router";
import TimeFrameControls from "./timeFrameControls";
Expand Down Expand Up @@ -51,7 +51,7 @@ describe("<TimeScaleDropdown>", function() {
let currentWindow: TimeWindow;

const setCurrentWindowFromTimeScale = (timeScale: TimeScale): void => {
const end = timeScale.windowEnd || moment.utc();
const end = timeScale.fixedWindowEnd || moment.utc();
currentWindow = {
start: moment(end).subtract(timeScale.windowSize),
end,
Expand All @@ -69,10 +69,10 @@ describe("<TimeScaleDropdown>", function() {

beforeEach(() => {
clock = sinon.useFakeTimers(new Date(2020, 5, 1, 9, 28, 30));
const timewindowState = new timewindow.TimeWindowState();
setCurrentWindowFromTimeScale(timewindowState.scale);
const timeScaleState = new timescale.TimeScaleState();
setCurrentWindowFromTimeScale(timeScaleState.scale);
state = {
currentScale: timewindowState.scale,
currentScale: timeScaleState.scale,
setTimeScale: () => {},
};
});
Expand All @@ -93,7 +93,7 @@ describe("<TimeScaleDropdown>", function() {
const expected: TimeScale = {
key: "Past 10 Minutes",
...defaultTimeScaleOptions["Past 10 Minutes"],
windowEnd: null,
fixedWindowEnd: false,
};
assert.deepEqual(wrapper.props().currentScale, expected);
});
Expand Down Expand Up @@ -143,7 +143,7 @@ describe("<TimeScaleDropdown>", function() {
};
const currentScale = {
...state.currentScale,
windowEnd: window.end,
fixedWindowEnd: window.end,
windowSize: moment.duration(
window.end.diff(window.start, "seconds"),
"seconds",
Expand Down Expand Up @@ -191,7 +191,7 @@ describe("<TimeScaleDropdown>", function() {
};
const currentTimeScale = {
...state.currentScale,
windowEnd: window.end,
fixedWindowEnd: window.end,
};
const arrows = generateDisabledArrows(window);
const wrapper = makeTimeScaleDropdown({
Expand All @@ -205,11 +205,21 @@ describe("<TimeScaleDropdown>", function() {

describe("timescale utils", (): void => {
describe("findClosestTimeScale", () => {
it("should found correctly time scale", () => {
it("should find the correct time scale", () => {
// `seconds` != window size of any of the default options, `startSeconds` not specified.
assert.deepEqual(findClosestTimeScale(defaultTimeScaleOptions, 15), {
...defaultTimeScaleOptions["Past 10 Minutes"],
key: "Custom",
});
// `seconds` != window size of any of the default options, `startSeconds` not specified.
assert.deepEqual(
findClosestTimeScale(
defaultTimeScaleOptions,
moment.duration(moment().daysInMonth() * 5, "days").asSeconds(),
),
{ ...defaultTimeScaleOptions["Past 2 Months"], key: "Custom" },
);
// `seconds` == window size of one of the default options, `startSeconds` not specified.
assert.deepEqual(
findClosestTimeScale(
defaultTimeScaleOptions,
Expand All @@ -220,6 +230,7 @@ describe("timescale utils", (): void => {
key: "Past 10 Minutes",
},
);
// `seconds` == window size of one of the default options, `startSeconds` not specified.
assert.deepEqual(
findClosestTimeScale(
defaultTimeScaleOptions,
Expand All @@ -230,12 +241,28 @@ describe("timescale utils", (): void => {
key: "Past 2 Weeks",
},
);
// `seconds` == window size of one of the default options, `startSeconds` is now.
assert.deepEqual(
findClosestTimeScale(
defaultTimeScaleOptions,
moment.duration(moment().daysInMonth() * 5, "days").asSeconds(),
defaultTimeScaleOptions["Past 10 Minutes"].windowSize.asSeconds(),
moment().unix(),
),
{ ...defaultTimeScaleOptions["Past 2 Months"], key: "Custom" },
{
...defaultTimeScaleOptions["Past 10 Minutes"],
key: "Past 10 Minutes",
},
);
// `seconds` == window size of one of the default options, `startSeconds` is in the past.
assert.deepEqual(
findClosestTimeScale(
defaultTimeScaleOptions,
defaultTimeScaleOptions["Past 10 Minutes"].windowSize.asSeconds(),
moment()
.subtract(1, "day")
.unix(),
),
{ ...defaultTimeScaleOptions["Past 10 Minutes"], key: "Custom" },
);
});
});
Expand Down
20 changes: 11 additions & 9 deletions pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,13 @@ export const defaultTimeScaleOptions: TimeScaleOptions = {
export const defaultTimeScaleSelected: TimeScale = {
...defaultTimeScaleOptions["Past 1 Hour"],
key: "Past 1 Hour",
windowEnd: null,
fixedWindowEnd: false,
};

export const toDateRange = (ts: TimeScale): [moment.Moment, moment.Moment] => {
const end = ts.windowEnd ? moment.utc(ts.windowEnd) : moment().utc();
const end = ts.fixedWindowEnd
? moment.utc(ts.fixedWindowEnd)
: moment().utc();
const start = moment.utc(end).subtract(ts.windowSize);
return [start, end];
};
Expand All @@ -100,22 +102,22 @@ export const findClosestTimeScale = (
Math.abs(seconds - b.windowSize.asSeconds()),
);

const firstTimeScaleOptionSeconds = data[0].windowSize.asSeconds();
const closestWindowSizeSeconds = data[0].windowSize.asSeconds();

// This logic covers the edge case where drag-to-timerange on a linegraph is of a duration
// that exactly matches one of the standard available time scales e.g. selecting June 1 at
// 0:00 to June 2 at 0:00 when the date is July 1 at 0:00 should return a custom timescale
// instead of past day.
if (startSeconds && firstTimeScaleOptionSeconds === seconds) {
const startWindow = moment()
.subtract(firstTimeScaleOptionSeconds, "seconds")
.unix();
if (startSeconds < startWindow) {
// If the start is specified, and the window size matches.
if (startSeconds && closestWindowSizeSeconds === seconds) {
// Check if the end is before now. If so, it is a custom time.
const end = moment.unix(startSeconds + seconds);
if (end < moment()) {
return { ...data[0], key: "Custom" };
}
}

return firstTimeScaleOptionSeconds === seconds
return closestWindowSizeSeconds === seconds
? data[0]
: { ...data[0], key: "Custom" };
};
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,6 @@ export const transactionDetails = {
export const timeScale: TimeScale = {
windowSize: moment.duration(1, "year"),
sampleSize: moment.duration(1, "day"),
windowEnd: moment.utc("2021.12.31"),
fixedWindowEnd: moment.utc("2021.12.31"),
key: "Custom",
};
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const columns: string[] = ["all"];
export const timeScale: TimeScale = {
windowSize: moment.duration(5, "day"),
sampleSize: moment.duration(5, "minutes"),
windowEnd: moment.utc("2021.08.12"),
fixedWindowEnd: moment.utc("2021.08.12"),
key: "Custom",
};
export const timestamp = new protos.google.protobuf.Timestamp({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import cn from "classnames";
import { Breadcrumbs } from "src/views/clusterviz/containers/map/breadcrumbs";
import NeedEnterpriseLicense from "src/views/clusterviz/containers/map/needEnterpriseLicense";
import NodeCanvasContainer from "src/views/clusterviz/containers/map/nodeCanvasContainer";
import TimeScaleDropdown from "src/views/cluster/containers/timescale";
import TimeScaleDropdown from "src/views/cluster/containers/timeScaleDropdownWithSearchParams";
import swapByLicense from "src/views/shared/containers/licenseSwap";
import { parseLocalityRoute } from "src/util/localities";
import { Loading } from "@cockroachlabs/cluster-ui";
Expand Down
6 changes: 3 additions & 3 deletions pkg/ui/workspaces/db-console/src/redux/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { hoverReducer, HoverState } from "./hover";
import { localSettingsReducer, LocalSettingsState } from "./localsettings";
import { metricsReducer, MetricsState } from "./metrics";
import { queryManagerReducer, QueryManagerState } from "./queryManager/reducer";
import { timeWindowReducer, TimeWindowState } from "./timewindow";
import { timeScaleReducer, TimeScaleState } from "./timeScale";
import { uiDataReducer, UIDataState } from "./uiData";
import { loginReducer, LoginAPIState } from "./login";
import rootSaga from "./sagas";
Expand All @@ -43,7 +43,7 @@ export interface AdminUIState {
metrics: MetricsState;
queryManager: QueryManagerState;
router: RouterState;
timewindow: TimeWindowState;
timeScale: TimeScaleState;
uiData: UIDataState;
login: LoginAPIState;
}
Expand All @@ -65,7 +65,7 @@ export function createAdminUIStore(historyInst: History<any>) {
metrics: metricsReducer,
queryManager: queryManagerReducer,
router: routerReducer,
timewindow: timeWindowReducer,
timeScale: timeScaleReducer,
uiData: uiDataReducer,
login: loginReducer,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Action } from "redux";
import { PayloadAction } from "src/interfaces/action";
import { google } from "@cockroachlabs/crdb-protobuf-client";
import IDuration = google.protobuf.IDuration;
import { TimeScale } from "src/redux/timewindow";
import { TimeScale } from "src/redux/timeScale";

export const CREATE_STATEMENT_DIAGNOSTICS_REPORT =
"cockroachui/statements/CREATE_STATEMENT_DIAGNOSTICS_REPORT";
Expand Down
Loading

0 comments on commit 9fa03c7

Please sign in to comment.