Reviewable status: complete! 0 of 0 LGTMs obtained

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPageConnected.tsx, line 72 at r1 (raw file):

          sqlStatsActions.updateDateRange({
            start: start.unix(),
            end: end.unix(),

The prior implementation stored start and end dates in UNIX format. I am instead implicitly storing windowEnd in UTC format, due to the default serialization of moment.Moment. I only convert to UNIX (ultimately to Long) when making the request. The implicit format feels weird, but is consistent with what the Metrics page currently does for its time picker.

pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts, line 51 at r1 (raw file):

The prior implementation would add 1 minute into the future to the end of the time scale, to account for potential lag. This is lost with my current implementation that naively uses TimeScaleDropdown. Does anyone (@maryliag?) know if this is okay, or if should I implement this buffer (probably in TimeScaleDropdown)?

pkg/ui/workspaces/db-console/src/views/statements/statementsPage.fixture.ts, line 0 at r1 (raw file):
Deleted this fixture file because there is no story.

Reviewed 22 of 24 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jocrl and @maryliag)

pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts, line 51 at r1 (raw file):

Don't need to worry about this. If we hit this again in the future we can make adjustments.

pkg/ui/workspaces/db-console/src/redux/statementsTimeScale.ts, line 20 at r1 (raw file):

  AdminUIState,
  TimeScale
>("statements_time_scale", localSettingsSelector, defaultTimeScaleSelected);

nit: can you use the same key we're using on cluster-ui? so timeScale/StatementsPage. So we can keep things aligned

pkg/ui/workspaces/db-console/src/redux/statements/statementsActions.ts, line 13 at r1 (raw file):

import { Action } from "redux";
import { PayloadAction } from "src/interfaces/action";
import { TimeScale } from "oss/src/redux/timewindow";

nit: remove the oss, start import with src/...

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @maryliag)

pkg/ui/workspaces/db-console/src/redux/statementsTimeScale.ts, line 20 at r1 (raw file):

done

Reviewed 4 of 24 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Azhng, @jocrl, and @maryliag)

-- commits, line 3 at r2:
nit: extra space in the commit msg ?

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts, line 150 at r2 (raw file):

  timeScale: {
    windowSize: moment.duration(5, "day"),
    sampleSize: moment.duration(5, "minutes"),

Hmm is the discrepancy between the windowSize and sampleSize possible in real life scenario? I think i'm just not entirely sure whats' the difference between windowSize and sampleSize here

pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.ts, line 66 at r2 (raw file):

  const { ts } = action.payload;
  yield put(
    // TODO(azhng): do we want to rename this into dataRange/SQLActivity?

do you mind update this todo 😛

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Azhng, @maryliag, and @xinhaoz)

-- commits, line 3 at r2:

done

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts, line 150 at r2 (raw file):

So I know windowSize is the straightforward one, it's the size of the time window.

For sampleSize, the comment string is: "The expected duration of individual samples for queries at this time scale.". I think this has to do with how fine-grained the information on the statements and transactions tables are. sampleSize is specified for all default options. In every case it's smaller than windowSize.

However, you raise a point that I'm pretty sure this isn't sent in the request. My IDE says that the only usage is here... which looks like it's just setting a floor on the sampleSize.

@xinhaoz, do you have an idea what sampleSize is for, or if the metrics page currently uses it? Or @maryliag if it's for some upcoming functionality?

pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.ts, line 66 at r2 (raw file):

Done, and renamed to timeScale/SQLActivity.
Any suggestions for the name when this also gets shared by the Metrics page? 😛

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Azhng, @jocrl, @maryliag, and @xinhaoz)

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts, line 150 at r2 (raw file):

Interesting, seems like it's used by the tsDB to represent the granularity of each sample point. Though I think it might be slightly difficult to adapt this for Statement / Transaction Page, since we don't have the aggregating capability in the backend yet. IIRC we do most of our aggregation in the frontend for now.

pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.ts, line 66 at r2 (raw file):

Hmm good question, timeScale/Global or timeScale/Observability ?

Reviewed 2 of 2 files at r2, 4 of 4 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Azhng and @xinhaoz)

pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.ts, line 66 at r2 (raw file):

I think we can still keep as timeScale/SQLActivity for that case, because the metrics there are also SQL Activity related

🎉

Reviewed 17 of 24 files at r1, 1 of 2 files at r2, 4 of 4 files at r3, 2 of 2 files at r4, 5 of 5 files at r5, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @Azhng and @xinhaoz)