server,ui: transactionDetail page now shows statement stats scoped by transaction fingerprint ID#71967
Conversation
57e2bc9 to
03cc84d
Compare
03cc84d to
a5a2175
Compare
matthewtodd
left a comment
There was a problem hiding this comment.
Nice! Just a few nits and a question.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @Azhng)
-- commits, line 5 at r1:
nit: s/transactionDetail/the transactionDetail/
-- commits, line 5 at r1:
nit: s/shows/showed
-- commits, line 9 at r1:
nit: s/transactionDetail/the transactionDetail/
pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx, line 147 at r1 (raw file):
s.key.key_data.transaction_fingerprint_id.comp( transactionFingerprintId, ) === 0,
Interesting, is there a reason not to use Long.equals here?
…d by transaction fingerprint ID Previously, the transactionDetail page showed statement statistics aggregated across **all** executions of that statement fingerprint. This led to confusing UX and the statement statistics were subsequently removed from transactionDetail page. This commit reintroduces statement statistics on the transactionDetail page. The statistics now are only aggregated across the statement fingerprints that are part of the selected transaction fingerprint. Resolves cockroachdb#59205 cockroachdb#65106 Release note (ui change): transactionDetail page now shows statement statistics scoped by the transaction fingeprint.
a5a2175 to
d798002
Compare
Azhng
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @matthewtodd)
Previously, matthewtodd (Matthew Todd) wrote…
nit: s/transactionDetail/the transactionDetail/
Done.
Previously, matthewtodd (Matthew Todd) wrote…
nit: s/shows/showed
Done.
Previously, matthewtodd (Matthew Todd) wrote…
nit: s/transactionDetail/the transactionDetail/
Done.
pkg/ui/workspaces/cluster-ui/src/transactionDetails/transactionDetails.tsx, line 147 at r1 (raw file):
Previously, matthewtodd (Matthew Todd) wrote…
Interesting, is there a reason not to use
Long.equalshere?
TIL, changed to use Long.equals
matthewtodd
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @Azhng)
|
TFTR! bors r=matthewtodd |
|
Build succeeded: |
Previously, transactionDetail page shows statement statistics aggregated
across all executions of that statement fingerprint. This led to
confusing UX and the statement statistics were subsequently removed
from transactionDetail page.
This commit reintroduces statement statistics on transactionDetail
page. The statistics now are only aggregated across the statement
fingerprints that are part of the selected transaction fingerprint.
Resolves #59205 #65106
Release note (ui change): transactionDetail page now shows statement
statistics scoped by the transaction fingeprint.
demo.mov