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.

Sign up for GitHub

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

Jump to bottom

De-duplicate state on timeScaleDropdown #75579

Closed
jocrl opened this issue Jan 26, 2022 · 0 comments · Fixed by #75586
Closed

De-duplicate state on timeScaleDropdown #75579

jocrl opened this issue Jan 26, 2022 · 0 comments · Fixed by #75586
Assignees
@jocrl
Labels
A-sql-console-general SQL Observability issues on the DB console spanning multiple areas. Includes Cockroach Cloud Console C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@jocrl
Copy link
Contributor

jocrl commented Jan 26, 2022 •
edited
Loading

There are two properties on timewindow state, scale and currentWindow. It seems like the selected start and end times could be derived from either property. Either by reading start and end from currentWindow, or by reading start = windowEnd - windowSizeandwindowEndfromscale`.

In general, having duplicate sources of truth risks bugs from discrepant update cycles, and confusion about when to read from which one. And if they are not duplicates of each other, it is unclear what the distinction is and thus when to update or read from each one.

currentWindow was introduced in May 2016, and windowEnd on scale was introduced in December 2016 (both authors no longer work at CRL). Today, multiple places in the code use the toDateRange function to derive start and end time from timescale (instead of using currentWindow). Further, the time picker is used in the statements and transactions page without storing the currentWindow state in redux at all; only scale is implemented. A few places use currentWindow, and one has a comment where currentWindow is used to fix a bug because scale was incorrect and out of sync.

It would be great if the two states could be de-duplicated without regressions (or made clearer what the distinction is, if any). This would also help #72128, to not have to also implement currentWindow.
@jocrl jocrl added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-observability A-sql-console-general SQL Observability issues on the DB console spanning multiple areas. Includes Cockroach Cloud Console labels Jan 26, 2022
@jocrl jocrl self-assigned this Jan 26, 2022
@jocrl jocrl mentioned this issue Jan 26, 2022
Merged
jocrl added a commit to jocrl/cockroach that referenced this issue Feb 15, 2022
@jocrl
ui: Always write to windowEnd on scale; read from scale for the metri…
a3d76b9 
…cs graph

Partially addresses cockroachdb#75579

Previously, `windowEnd` on `scale` was not always written to, meaning that it
was not always safe/correct to read off the `scale` property. `currentWindow`
wasn't always safe to use either.

This commit modifies `windowEnd` to always be set, by making it an non-optional
property and letting Typescript do the enforcement. It also modifies the
metrics page graph to read from `scale` instead of `currentWindow`

There was a comment in `metricDataProvider/index.tsx#L258` where `currentWindow`
is used to fix a bug on the metrics page because `scale` was incorrect and out
of sync. This commit does not regress that drag-to-zoom bug.

Side note about the bug: at the code state when the cockroachdb#70594 bug was fixed, the
`windowEnd` property was not set (this commit changes `linegraph/index.tsx` to
set it). Yet, even without changing `linegraph/index.tsx` to set `windowEnd`,
reading the start and end times from `scale` did not recreate the drag-to-zoom
granularity bug. However, reading from `scale` without setting `windowEnd` does
cause a separate major bug in that the timescale endpoint is always queried
with `now` as an end time instead of the time that the user selected and is
displayed. This PR does not have this bug.

Release note: None
jocrl added a commit to jocrl/cockroach that referenced this issue Feb 15, 2022
@jocrl
ui: Make currentWindow.end the single source of truth
aae2684 
Partially addresses cockroachdb#75579

Previously, there were two properties on `timewindow` state: `scale` and
`currentWindow`. It turns out that:
1) `currentWindow` is not used in cluster-ui
2) `currentWindow.end` differs from `scale.windowEnd` in that the
former is always a specific time which is used to do polling in the DB console
metrics page

This commit thus:
- deletes the `currentWindow`, `scaleChanged`, and `useTimeRange` states in
  cluster-ui
- nests the `currentWindow`, `scaleChanged`, and `useTimeRange` states under the
  `metricsTime` property in db-console

For a more in-depth description, see the PR description.

Release note: None
jocrl added a commit to jocrl/cockroach that referenced this issue Feb 15, 2022
@jocrl
ui: Rename variables and add comments
e91b615 
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
jocrl added a commit to jocrl/cockroach that referenced this issue Feb 17, 2022
@jocrl
ui: Make currentWindow.end the single source of truth
25f2039 
Partially addresses cockroachdb#75579

Previously, there were two properties on `timewindow` state: `scale` and
`currentWindow`. It turns out that:
1) `currentWindow` is not used in cluster-ui
2) `currentWindow.end` differs from `scale.windowEnd` in that the
former is always a specific time which is used to do polling in the DB console
metrics page

This commit thus:
- deletes the `currentWindow`, `scaleChanged`, and `useTimeRange` states in
  cluster-ui
- nests the `currentWindow`, `scaleChanged`, and `useTimeRange` states under the
  `metricsTime` property in db-console

For a more in-depth description, see the PR description.

Release note: None
jocrl added a commit to jocrl/cockroach that referenced this issue Feb 17, 2022
@jocrl
ui: Rename variables and add comments
ad6e433 
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
jocrl added a commit to jocrl/cockroach that referenced this issue Feb 24, 2022
@jocrl
ui: Rename variables and add comments
5b5189b 
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
craig bot pushed a commit that referenced this issue Feb 25, 2022
@jocrl @rafiss
Merge #75586 #77009
08bbadf 
75586: ui: De-duplicate and clarify time selection state r=jocrl a=jocrl

Resolves #75579. This is an entirely internal refactor. https://github.com/cockroachlabs/managed-service/pull/7998 makes companion changes in managed-service.

Suggestions from comments from this PR review have been separated into [this issue](#76593).

# Commits

## Commit 1 ui: Always write to `windowEnd` on `scale`; read from `scale` for the metrics graph.

Partially addresses #75579

Previously, `windowEnd` on `scale` was not always written to, meaning that it was not always safe/correct to read off the `scale` property. `currentWindow` wasn't always safe to use either.

This commit modifies `windowEnd` to always be set, by making it an non-optional property and letting Typescript do the enforcement. It also modifies the metrics page graph to read from `scale` instead of `currentWindow`

There was a [comment](https://github.com/cockroachdb/cockroach/blob/master/pkg/ui/workspaces/db-console/src/views/shared/containers/metricDataProvider/index.tsx#L258) where `currentWindow` is used to fix a bug on the metrics page because `scale` was incorrect and out of sync. This commit does not regress that drag-to-zoom bug.

Side note about the bug: at the code state when the #70594 bug was fixed, the `windowEnd` property was not set (this commit changes `linegraph/index.tsx` to set it). Yet, even without changing `linegraph/index.tsx` to set `windowEnd`, reading the start and end times from `scale` did not recreate the drag-to-zoom granularity bug. However, reading from `scale` without setting `windowEnd` does cause a separate major bug in that the timescale endpoint is always queried with `now` as an end time instead of the time that the user selected and is displayed. This PR does not have this bug.

Release note: None

## Commit 2 ui: Make `currentWindow.end` the single source of truth

Partially addresses #75579

Previously, there are [two properties](https://github.com/cockroachdb/cockroach/blob/f18fe733c724cf51bffcde0535caa5d4bab7fa6c/pkg/ui/workspaces/db-console/src/redux/timewindow.ts#L57) on `timewindow` state, `scale` and `currentWindow`. They seemed like duplicates (see #75579 issue for further explanation). It was unclear what the distinction was, if any.

1) `currentWindow` is not used in cluster-ui
- In cluster-ui, the `currentWindow` property on state is only ever read from in [this test](https://github.com/cockroachdb/cockroach/blob/f18fe733c724cf51bffcde0535caa5d4bab7fa6c/pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timescale.spec.tsx#L73). Further, the local variable that it is assigned to is immediately overwritten in the line right after. So to say, cluster-ui does not use the `currentWindow` state, even if there are a lot of local variables that are named `currentWindow`. `TimeWindow` is used as a type, but is not used on state.

2) `currentWindow.end` differs from `scale.windowEnd` in that the former is always a specific time which is used to do polling in the db console metrics page
- Users may select that they want the end time to be "now", or a specific end time. When "now" is selected, `scale.windowEnd` is `null`, and `currentWindow.end` is a specific time, e.g. 3.51pm on a Monday. `currentWindow.end` is continually updated according to the `windowValid` frequency on the closest `DefaultTimeScaleOption`. The continual update of `currentWindow.end` is used to poll for new data for graphs on the metrics page.
- There are two other places where the time selection component is used: the serverless metrics page (which is a completely different component from the db console metrics page), and the statements and transactions pages. Both of these other usages use completely different polling mechanisms that do not rely on `currentWindow.end` (and actually, also don't use the `windowValid` frequency).
- So to say, `currentWindow.end` is only used for the db console metrics page. `scaleChanged` and `useTimeRange` are also only used for the db console metrics page, and help with bookkeeping around updating `currentWindow.end`.

This commit thus
- deletes the `currentWindow`, `scaleChanged`, and `useTimeRange` states in cluster-ui
- nests the `currentWindow`, `scaleChanged`, and `useTimeRange` states under the `metricsTime` property in db-console

Release note: None

## Commit 3 ui: Rename variables and add comments

Finishes addressing #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

77009: pgwire: fix flaky AOST test r=otan a=rafiss

fixes #76965

Change the test so it doesn't rely on sleeps, and instead uses
cluster-provided timestamps.

Release note: None

Co-authored-by: Josephine Lee <josephine@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
@craig craig bot closed this as completed in #75586 Feb 25, 2022
maryliag pushed a commit to maryliag/cockroach that referenced this issue Feb 28, 2022
@jocrl @maryliag
ui: Always write to windowEnd on scale; read from scale for the metri…
54dbdec 
…cs graph

Partially addresses cockroachdb#75579

Previously, `windowEnd` on `scale` was not always written to, meaning that it
was not always safe/correct to read off the `scale` property. `currentWindow`
wasn't always safe to use either.

This commit modifies `windowEnd` to always be set, by making it an non-optional
property and letting Typescript do the enforcement. It also modifies the
metrics page graph to read from `scale` instead of `currentWindow`

There was a comment in `metricDataProvider/index.tsx#L258` where `currentWindow`
is used to fix a bug on the metrics page because `scale` was incorrect and out
of sync. This commit does not regress that drag-to-zoom bug.

Side note about the bug: at the code state when the cockroachdb#70594 bug was fixed, the
`windowEnd` property was not set (this commit changes `linegraph/index.tsx` to
set it). Yet, even without changing `linegraph/index.tsx` to set `windowEnd`,
reading the start and end times from `scale` did not recreate the drag-to-zoom
granularity bug. However, reading from `scale` without setting `windowEnd` does
cause a separate major bug in that the timescale endpoint is always queried
with `now` as an end time instead of the time that the user selected and is
displayed. This PR does not have this bug.

Release note: None
maryliag pushed a commit to maryliag/cockroach that referenced this issue Feb 28, 2022
@jocrl @maryliag
ui: Make currentWindow.end the single source of truth
931df12 
Partially addresses cockroachdb#75579

Previously, there were two properties on `timewindow` state: `scale` and
`currentWindow`. It turns out that:
1) `currentWindow` is not used in cluster-ui
2) `currentWindow.end` differs from `scale.windowEnd` in that the
former is always a specific time which is used to do polling in the DB console
metrics page

This commit thus:
- deletes the `currentWindow`, `scaleChanged`, and `useTimeRange` states in
  cluster-ui
- nests the `currentWindow`, `scaleChanged`, and `useTimeRange` states under the
  `metricsTime` property in db-console

For a more in-depth description, see the PR description.

Release note: None
maryliag pushed a commit to maryliag/cockroach that referenced this issue Feb 28, 2022
@jocrl @maryliag
ui: Rename variables and add comments
f6e720b 
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
RajivTS pushed a commit to RajivTS/cockroach that referenced this issue Mar 6, 2022
@jocrl @RajivTS
ui: Always write to windowEnd on scale; read from scale for the metri…
8a2ba6e 
…cs graph

Partially addresses cockroachdb#75579

Previously, `windowEnd` on `scale` was not always written to, meaning that it
was not always safe/correct to read off the `scale` property. `currentWindow`
wasn't always safe to use either.

This commit modifies `windowEnd` to always be set, by making it an non-optional
property and letting Typescript do the enforcement. It also modifies the
metrics page graph to read from `scale` instead of `currentWindow`

There was a comment in `metricDataProvider/index.tsx#L258` where `currentWindow`
is used to fix a bug on the metrics page because `scale` was incorrect and out
of sync. This commit does not regress that drag-to-zoom bug.

Side note about the bug: at the code state when the cockroachdb#70594 bug was fixed, the
`windowEnd` property was not set (this commit changes `linegraph/index.tsx` to
set it). Yet, even without changing `linegraph/index.tsx` to set `windowEnd`,
reading the start and end times from `scale` did not recreate the drag-to-zoom
granularity bug. However, reading from `scale` without setting `windowEnd` does
cause a separate major bug in that the timescale endpoint is always queried
with `now` as an end time instead of the time that the user selected and is
displayed. This PR does not have this bug.

Release note: None
RajivTS pushed a commit to RajivTS/cockroach that referenced this issue Mar 6, 2022
@jocrl @RajivTS
ui: Make currentWindow.end the single source of truth
f2f813a 
Partially addresses cockroachdb#75579

Previously, there were two properties on `timewindow` state: `scale` and
`currentWindow`. It turns out that:
1) `currentWindow` is not used in cluster-ui
2) `currentWindow.end` differs from `scale.windowEnd` in that the
former is always a specific time which is used to do polling in the DB console
metrics page

This commit thus:
- deletes the `currentWindow`, `scaleChanged`, and `useTimeRange` states in
  cluster-ui
- nests the `currentWindow`, `scaleChanged`, and `useTimeRange` states under the
  `metricsTime` property in db-console

For a more in-depth description, see the PR description.

Release note: None
RajivTS pushed a commit to RajivTS/cockroach that referenced this issue Mar 6, 2022
@jocrl @RajivTS
ui: Rename variables and add comments
9fa03c7 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jocrl jocrl

Labels
A-sql-console-general SQL Observability issues on the DB console spanning multiple areas. Includes Cockroach Cloud Console C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ui: De-duplicate and clarify time selection state jocrl/cockroach
1 participant
@jocrl