ui: new charts to statement details page #82426
Conversation
|
This change is |
maryliag
force-pushed
the
new-charts-details
branch
8 times, most recently
from
d8b23ca to
ccd8612
Compare
|
|
||
| stats.forEach(function(stat: statementStatisticsPerAggregatedTs) { | ||
| ts.push(TimestampToNumber(stat.aggregated_ts) * 1e3); | ||
| read.push(stat.stats.rows_read.mean); |
There was a problem hiding this comment.
Does this row need a 1e3 multiplier, too?
There was a problem hiding this comment.
Or maybe the rows_written one, below, doesn't?
| // by the Apache License, Version 2.0, included in the file | ||
| // licenses/APL.txt. | ||
|
|
||
| Object.defineProperty(window, "matchMedia", { |
There was a problem hiding this comment.
Maybe a comment about what this is / why it's here / how to use it?
maryliag
force-pushed
the
new-charts-details
branch
from
ccd8612 to
ac31f24
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @Annebirzin and @matthewtodd)
pkg/ui/workspaces/cluster-ui/src/statementDetails/timeseriesUtils.ts line 42 at r1 (raw file):
Previously, matthewtodd (Matthew Todd) wrote…
Or maybe the
rows_writtenone, below, doesn't?
oops, this multiplier should not be there. Removed from below
pkg/ui/workspaces/cluster-ui/src/test-utils/matchMedia.mock.js line 11 at r1 (raw file):
Previously, matthewtodd (Matthew Todd) wrote…
Maybe a comment about what this is / why it's here / how to use it?
Comment added.
Regarding the "how to use it": once this file was added to the jest config file, there isn't anything else that needs to be done. If you create a new test that uses addEventListener for example, you wouldn't need to do anything to make it work, so I didn't add anything in particular about the usage in the comment. Let me know if looks good like this
maryliag
force-pushed
the
new-charts-details
branch
2 times, most recently
from
530f95b to
2d73fbf
Compare
maryliag
force-pushed
the
new-charts-details
branch
from
2d73fbf to
2e44786
Compare
maryliag
force-pushed
the
new-charts-details
branch
2 times, most recently
from
a23cae3 to
abea119
Compare
The overview for a statement details page now shows charts instead of just the mean value for: - Execution and Planning Time - Rows Processed - Execution Retries - Execution Count - Contention Fixes cockroachdb#74517 Fixes cockroachdb#81153 This commit also introduces mock for matchMedia and canvas used for testing with jest. Release note (ui change): Statement Details page now shows charts for: Execution and Planning Time, Rows Processed, Execution Retries, Execution Count and Contention.
maryliag
force-pushed
the
new-charts-details
branch
from
abea119 to
c3eda02
Compare
|
bors r+ |
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from c3eda02 to blathers/backport-release-22.1-82426: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
The overview for a statement details page now
shows charts instead of just the mean value for:
https://www.loom.com/share/2b6d0311e61a433d81ad37b8b62fba7d
Fixes #74517
Release note (ui change): Statement Details page now shows charts
for: Execution and Planning Time, Rows Processed, Execution Retries,
Execution Count and Contention.