roachpb, server, ui: add fingerprint ID to details pages#91885
roachpb, server, ui: add fingerprint ID to details pages#91885craig[bot] merged 3 commits intocockroachdb:masterfrom
Conversation
The format.js file was duplicated on db console, making it hard to maintain. This commit replaces all usages of the format functions on db-console package, to the ones from cluster-ui. Epic: None Release note: None
Previously, some fingerprint IDs in hex format would have a leading 0, that was not being returning from the DB calls. This was causing confusion when seeing the value on the UI and trying to find the same fringerprint on our tables. This commits checks the hex values used and add the missing leading zeros back. Part Of: cockroachdb#91763 Release note (bug fix): Add leading zeros to fingerprint IDs with less than 16 characters.
0ca0b8a to
be2cf04
Compare
j82w
left a comment
There was a problem hiding this comment.
Reviewed 16 of 16 files at r1, 5 of 6 files at r2, 7 of 7 files at r3, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @dongniwang and @maryliag)
pkg/ui/workspaces/cluster-ui/src/util/format.ts line 265 at r2 (raw file):
return ""; } if (s?.length === 16) {
Is this check needed? The while will just get skipped if it is 16.
pkg/ui/workspaces/cluster-ui/src/util/format.ts line 261 at r3 (raw file):
// CheckHexValue adds the leading 0 on strings with hex value that // have a length < 16. export function CheckHexValue(s: string): string {
Should it be named more like FixFingerprintHexValue? Can the comment also be updated to say that .toString(16) removes the 0s and it needs to be added back to match the sql tables?
Previously, there was no easy way to get a statement or fingerprint id from their respective details pages, allowing then the value to be found on our internal tables using cli. This commit adds the fingerprint id on the response of the statement details page. It also adds the fingeprint ID to the Statement Details page and Transaction Details page. Fixes cockroachdb#91763 Release note (ui change): Add fingerprint ID in hex format to the Statement Details page and Transaction Details page.
be2cf04 to
66730b8
Compare
maryliag
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @dongniwang and @j82w)
pkg/ui/workspaces/cluster-ui/src/util/format.ts line 265 at r2 (raw file):
Previously, j82w wrote…
Is this check needed? The while will just get skipped if it is 16.
Good point! I was doing it slightly different initially and forgot to remove that part! thanks for the catch!
pkg/ui/workspaces/cluster-ui/src/util/format.ts line 261 at r3 (raw file):
Previously, j82w wrote…
Should it be named more like FixFingerprintHexValue? Can the comment also be updated to say that .toString(16) removes the 0s and it needs to be added back to match the sql tables?
Done
|
TFTR! |
|
Build succeeded: |
Commit 1
ui: use format from cluster-ui
The format.js file was duplicated on db console,
making it hard to maintain.
This commit replaces all usages of the format
functions on db-console package, to the ones
from cluster-ui.
Epic: None
Release note: None
Commit 2
ui: add leading zeros to hex value with less than 16 chars
Previously, some fingerprint IDs in hex format would have
a leading 0, that was not being returning from the DB calls.
This was causing confusion when seeing the value on the UI
and trying to find the same fingerprint on our tables.
This commits checks the hex values used and add the missing
leading zeros back.
Part Of: #91763
Release note (bug fix): Add leading zeros to fingerprint IDs
with less than 16 characters.
Commit 3
roachpb, server, ui: add fingerprint ID to details pages
Previously, there was no easy way to get a statement or
fingerprint id from their respective details pages,
allowing then the value to be found on our internal tables
using cli.
This commit adds the fingerprint id on the response of
the statement details page.
It also adds the fingerprint ID to the Statement Details
page and Transaction Details page.
Fixes #91763
Release note (ui change): Add fingerprint ID in hex format
to the Statement Details page and Transaction Details page.