Skip to content

Commit

Permalink
ui: fix timescale for rolling window
Browse files Browse the repository at this point in the history
The change introduced on #105157 had an undesired
change on the Metrics page. We want to show the end period
of the select, but when the fixed window end for that this
caused the Metrics page to stop loading new data.
We want the metrics page to keep updating when any of the
rolling windows is selected, and we also want to know
the time a time period was requested, so it can be used
to provide more information on SQL Activity pages.

This commit remove the setting of the fixedWindowEnd, since
that was not the best approach and instead creates a value
on local storage for the requestTime, that can be used to create
the label for the timescale, without interferring on
Metrics page (or any other that have an automatic update).

Epic: none

Release note (bug fix): Fix the Metrics page that was not
updating automatically on rolling window options.
  • Loading branch information
maryliag committed Jul 11, 2023
1 parent 92d5d0e commit 2c12834
Show file tree
Hide file tree
Showing 19 changed files with 124 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import { TestStoreProvider } from "src/test-utils";
import { StatementDiagnosticsReport } from "../../api";
import moment from "moment-timezone";
import { SortedTable } from "src/sortedtable";
import { TimeScale } from "src/timeScaleDropdown";

const activateDiagnosticsRef = { current: { showModalFor: jest.fn() } };
const ts = {
const ts: TimeScale = {
windowSize: moment.duration(20, "day"),
sampleSize: moment.duration(5, "minutes"),
fixedWindowEnd: moment.utc("2023.01.5"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,7 @@ export const getStatementDetailsPropsFixture = (
"3": "gcp-us-west1",
"4": "gcp-europe-west1",
},
requestTime: moment.utc("2021.12.12"),
refreshStatementDetails: noop,
refreshStatementDiagnosticsRequests: noop,
refreshNodes: noop,
Expand All @@ -860,6 +861,7 @@ export const getStatementDetailsPropsFixture = (
diagnosticsReports: [],
dismissStatementDiagnosticsAlertMessage: noop,
onTimeScaleChange: noop,
onRequestTimeChange: noop,
createStatementDiagnosticsReport: noop,
uiConfig: {
showStatementDiagnosticsLink: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export interface StatementDetailsDispatchProps {
ascending: boolean,
) => void;
onBackToStatementsClick?: () => void;
onRequestTimeChange: (t: moment.Moment) => void;
}

export interface StatementDetailsStateProps {
Expand All @@ -160,6 +161,7 @@ export interface StatementDetailsStateProps {
hasViewActivityRedactedRole?: UIConfigState["hasViewActivityRedactedRole"];
hasAdminRole?: UIConfigState["hasAdminRole"];
statementFingerprintInsights?: StmtInsightEvent[];
requestTime: moment.Moment;
}

export type StatementDetailsOwnProps = StatementDetailsDispatchProps &
Expand Down Expand Up @@ -247,12 +249,10 @@ export class StatementDetails extends React.Component<
this.props.diagnosticsReports.length > 0;

changeTimeScale = (ts: TimeScale): void => {
if (ts.key !== "Custom") {
ts.fixedWindowEnd = moment();
}
if (this.props.onTimeScaleChange) {
this.props.onTimeScaleChange(ts);
}
this.props.onRequestTimeChange(moment());
};

refreshStatementDetails = (): void => {
Expand Down Expand Up @@ -701,14 +701,17 @@ export class StatementDetails extends React.Component<
<TimeScaleDropdown
options={timeScale1hMinOptions}
currentScale={this.props.timeScale}
setTimeScale={this.props.onTimeScaleChange}
setTimeScale={this.changeTimeScale}
/>
</PageConfigItem>
</PageConfig>
<p className={timeScaleStylesCx("time-label", "label-margin")}>
Showing aggregated stats from{" "}
<span className={timeScaleStylesCx("bold")}>
<FormattedTimescale ts={this.props.timeScale} />
<FormattedTimescale
ts={this.props.timeScale}
requestTime={moment(this.props.requestTime)}
/>
</span>
</p>
<section className={cx("section")}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
} from "src/api";
import { TimeScale } from "../timeScaleDropdown";
import { getMatchParamByName, statementAttr } from "src/util";
import { selectRequestTime } from "src/statementsPage/statementsPage.selectors";

// For tenant cases, we don't show information about node, regions and
// diagnostics.
Expand All @@ -76,6 +77,7 @@ const mapStateToProps = (state: AppState, props: RouteComponentProps) => {
isTenant: selectIsTenant(state),
hasViewActivityRedactedRole: selectHasViewActivityRedactedRole(state),
hasAdminRole: selectHasAdminRole(state),
requestTime: selectRequestTime(state),
statementFingerprintInsights: selectStatementFingerprintInsights(
state,
props,
Expand Down Expand Up @@ -169,6 +171,14 @@ const mapDispatchToProps = (
tableName,
}),
),
onRequestTimeChange: (t: moment.Moment) => {
dispatch(
localStorageActions.update({
key: "requestTime/StatementsPage",
value: t,
}),
);
},
onBackToStatementsClick: () =>
dispatch(
analyticsActions.track({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
ascending: false,
columnTitle: "executionCount",
},
requestTime: moment.utc("2021.12.12"),
search: "",
filters: {
app: "",
Expand Down Expand Up @@ -398,6 +399,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
onChangeLimit: noop,
onChangeReqSort: noop,
onApplySearchCriteria: noop,
onRequestTimeChange: noop,
};

export const statementsPagePropsWithRequestError: StatementsPageProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ export const selectSortSetting = createSelector(
localStorage => localStorage["sortSetting/StatementsPage"],
);

export const selectRequestTime = createSelector(
localStorageSelector,
localStorage => localStorage["requestTime/StatementsPage"],
);

export const selectFilters = createSelector(
localStorageSelector,
localStorage => localStorage["filters/StatementsPage"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ export interface StatementsPageDispatchProps {
onChangeLimit: (limit: number) => void;
onChangeReqSort: (sort: SqlStatsSortType) => void;
onApplySearchCriteria: (ts: TimeScale, limit: number, sort: string) => void;
onRequestTimeChange: (t: moment.Moment) => void;
}
export interface StatementsPageStateProps {
statementsResponse: RequestState<SqlStatsResponse>;
Expand All @@ -150,6 +151,7 @@ export interface StatementsPageStateProps {
hasViewActivityRedactedRole?: UIConfigState["hasViewActivityRedactedRole"];
hasAdminRole?: UIConfigState["hasAdminRole"];
stmtsTotalRuntimeSecs: number;
requestTime: moment.Moment;
}

export interface StatementsPageState {
Expand Down Expand Up @@ -262,9 +264,6 @@ export class StatementsPage extends React.Component<
};

changeTimeScale = (ts: TimeScale): void => {
if (ts.key !== "Custom") {
ts.fixedWindowEnd = moment();
}
this.setState(prevState => ({
...prevState,
timeScale: ts,
Expand All @@ -280,8 +279,6 @@ export class StatementsPage extends React.Component<
this.props.onChangeReqSort(this.state.reqSortSetting);
}

// Force an update on TimeScale to update the fixedWindowEnd
this.changeTimeScale(this.state.timeScale);
if (this.props.timeScale !== this.state.timeScale) {
this.props.onTimeScaleChange(this.state.timeScale);
}
Expand All @@ -292,6 +289,7 @@ export class StatementsPage extends React.Component<
getSortLabel(this.state.reqSortSetting, "Statement"),
);
}
this.props.onRequestTimeChange(moment());
this.refreshStatements();
const ss: SortSetting = {
ascending: false,
Expand Down Expand Up @@ -586,7 +584,12 @@ export class StatementsPage extends React.Component<
isSelectedColumn(userSelectedColumnsToShow, c),
);

const period = <FormattedTimescale ts={this.props.timeScale} />;
const period = (
<FormattedTimescale
ts={this.props.timeScale}
requestTime={moment(this.props.requestTime)}
/>
);
const sortSettingLabel = getSortLabel(
this.props.reqSortSetting,
"Statement",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
selectSortSetting,
selectFilters,
selectSearch,
selectRequestTime,
} from "./statementsPage.selectors";
import {
selectTimeScale,
Expand Down Expand Up @@ -96,6 +97,7 @@ export const ConnectedStatementsPage = withRouter(
search: selectSearch(state),
sortSetting: selectSortSetting(state),
limit: selectStmtsPageLimit(state),
requestTime: selectRequestTime(state),
reqSortSetting: selectStmtsPageReqSort(state),
stmtsTotalRuntimeSecs:
state.adminUI?.statements?.data?.stmts_total_runtime_secs ?? 0,
Expand Down Expand Up @@ -220,6 +222,14 @@ export const ConnectedStatementsPage = withRouter(
}),
);
},
onRequestTimeChange: (t: moment.Moment) => {
dispatch(
localStorageActions.update({
key: "requestTime/StatementsPage",
value: t,
}),
);
},
onStatementClick: () =>
dispatch(
analyticsActions.track({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ export type LocalStorageState = {
"showSetting/JobsPage": string;
[LocalStorageKeys.DB_DETAILS_VIEW_MODE]: ViewMode;
[LocalStorageKeys.ACTIVE_EXECUTIONS_IS_AUTOREFRESH_ENABLED]: boolean;
"requestTime/StatementsPage": moment.Moment;
"requestTime/TransactionsPage": moment.Moment;
};

type Payload = {
Expand Down Expand Up @@ -283,6 +285,8 @@ const initialState: LocalStorageState = {
LocalStorageKeys.ACTIVE_EXECUTIONS_IS_AUTOREFRESH_ENABLED,
),
) || defaultIsAutoRefreshEnabledSetting,
"requestTime/StatementsPage": null,
"requestTime/TransactionsPage": null,
};

const localStorageSlice = createSlice({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import { toRoundedDateRange } from "./utils";
import { TimeScale } from "./timeScaleTypes";
import { Timezone } from "src/timestamp";

export const FormattedTimescale = (props: { ts: TimeScale }) => {
export const FormattedTimescale = (props: {
ts: TimeScale;
requestTime?: moment.Moment;
}) => {
const timezone = useContext(TimezoneContext);

const [start, end] = toRoundedDateRange(props.ts);
Expand All @@ -30,8 +33,8 @@ export const FormattedTimescale = (props: { ts: TimeScale }) => {
omitDayFormat || startEndOnSameDay ? "" : endTz.format(dateFormat);
const timeStart = startTz.format(timeFormat);
const timeEnd =
props.ts.key !== "Custom" && props.ts.fixedWindowEnd
? props.ts.fixedWindowEnd.tz(timezone).format(timeFormat)
props.ts.key !== "Custom" && props.requestTime?.isValid()
? props.requestTime.tz(timezone).format(timeFormat)
: endTz.format(timeFormat);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export interface TransactionDetailsStateProps {
transactionInsights: TxnInsightEvent[];
hasAdminRole?: UIConfigState["hasAdminRole"];
txnStatsResp: RequestState<SqlStatsResponse>;
requestTime: moment.Moment;
}

export interface TransactionDetailsDispatchProps {
Expand All @@ -124,6 +125,7 @@ export interface TransactionDetailsDispatchProps {
refreshUserSQLRoles: () => void;
refreshTransactionInsights: (req: TxnInsightsRequest) => void;
onTimeScaleChange: (ts: TimeScale) => void;
onRequestTimeChange: (t: moment.Moment) => void;
}

export type TransactionDetailsProps = TransactionDetailsStateProps &
Expand Down Expand Up @@ -224,12 +226,10 @@ export class TransactionDetails extends React.Component<
};

changeTimeScale = (ts: TimeScale): void => {
if (ts.key !== "Custom") {
ts.fixedWindowEnd = moment();
}
if (this.props.onTimeScaleChange) {
this.props.onTimeScaleChange(ts);
}
this.props.onRequestTimeChange(moment());
};

refreshData = (): void => {
Expand Down Expand Up @@ -289,7 +289,12 @@ export class TransactionDetails extends React.Component<
const error = this.props.txnStatsResp?.error;
const { latestTransactionText, statements } = this.state;
const transactionStats = transaction?.stats_data?.stats;
const period = <FormattedTimescale ts={this.props.timeScale} />;
const period = (
<FormattedTimescale
ts={this.props.timeScale}
requestTime={moment(this.props.requestTime)}
/>
);

return (
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import { connect } from "react-redux";
import { withRouter } from "react-router-dom";
import { Dispatch } from "redux";

import { actions as localStorageActions } from "src/store/localStorage";
import { AppState, uiConfigActions } from "src/store";
import { actions as nodesActions } from "../store/nodes";
import { actions as sqlStatsActions } from "src/store/sqlStats";
Expand Down Expand Up @@ -42,6 +42,7 @@ import { StatementsRequest } from "src/api/statementsApi";
import { txnFingerprintIdAttr, getMatchParamByName } from "../util";
import { TimeScale } from "../timeScaleDropdown";
import { actions as analyticsActions } from "../store/analytics";
import { selectRequestTime } from "src/transactionsPage/transactionsPage.selectors";

const mapStateToProps = (
state: AppState,
Expand All @@ -61,6 +62,7 @@ const mapStateToProps = (
hasAdminRole: selectHasAdminRole(state),
limit: selectTxnsPageLimit(state),
reqSortSetting: selectTxnsPageReqSort(state),
requestTime: selectRequestTime(state),
};
};

Expand Down Expand Up @@ -88,6 +90,14 @@ const mapDispatchToProps = (
refreshTransactionInsights: (req: TxnInsightsRequest) => {
dispatch(transactionInsights.refresh(req));
},
onRequestTimeChange: (t: moment.Moment) => {
dispatch(
localStorageActions.update({
key: "requestTime/StatementsPage",
value: t,
}),
);
},
});

export const TransactionDetailsPageConnected = withRouter<any, any>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ export const selectSortSetting = createSelector(
localStorage => localStorage["sortSetting/TransactionsPage"],
);

export const selectRequestTime = createSelector(
localStorageSelector,
localStorage => localStorage["requestTime/TransactionsPage"],
);

export const selectFilters = createSelector(
localStorageSelector,
localStorage => localStorage["filters/TransactionsPage"],
Expand Down
Loading

0 comments on commit 2c12834

Please sign in to comment.