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.

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

Add statement/transaction fingerprint ID to the overview page #78509

Closed
kevin-v-ngo opened this issue Mar 25, 2022 · 3 comments · Fixed by #85464
Closed

Add statement/transaction fingerprint ID to the overview page #78509

kevin-v-ngo opened this issue Mar 25, 2022 · 3 comments · Fixed by #85464
Assignees
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

@kevin-v-ngo
Copy link

kevin-v-ngo commented Mar 25, 2022

This will facilitate a better searchability and filtering experience in the overview pages once a text-based search has narrowed down the results. This is for the user flow where the ID has already be identified.

It will also provide clarity throughout the troubleshooting journey when diving into fingerprint detail views.

The ID should be hex-based and hidden by default.

Jira issue: CRDB-14168

@kevin-v-ngo kevin-v-ngo added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-sql-console-general SQL Observability issues on the DB console spanning multiple areas. Includes Cockroach Cloud Console labels Mar 25, 2022
@maryliag
Copy link
Contributor

maryliag commented Jul 4, 2022

what is the format we want to display here, also should we have this column as hidden by default?

@maryliag
Copy link
Contributor

maryliag commented Jul 5, 2022

Also, the search should also look into fingerprint ID

@kevin-v-ngo
Copy link
Author

what is the format we want to display here, also should we have this column as hidden by default?

I'll update the issue but let's hide it by default for now. Let's also encode it to 'hex' for better readability. Similar to what we did for the contention queries/views.

craig bot pushed a commit that referenced this issue Aug 26, 2022
85464: ui: add Transaction and Statement fingerprint id r=maryliag a=j82w

This adds the Transaction and Statement fingerprint id
to SQL Activity Statements overview page. The columns
are hidden by default.

closes #78509

<img width="1337" alt="Screen Shot 2022-08-03 at 7 14 21 AM" src="https://user-images.githubusercontent.com/8868107/182594974-73bea276-0fb0-4cd4-a106-60e31962b1b9.png">

<img width="1341" alt="Screen Shot 2022-08-03 at 7 21 43 AM" src="https://user-images.githubusercontent.com/8868107/182596064-7a02e7da-3e07-4616-9be9-1a807b87b33d.png">

Release justification: Category 2: Bug fixes and
low-risk updates to new functionality

Release note (ui change): Adds Transaction and Statement fingerprint
id to correlating SQL Activity overview pages. New columns are
hidden by default.

86483: kvserver: decrease trace verbosity for change replicas db operations r=AlexTalks a=AlexTalks

While we introduced logging of traces from replicate queue processing
in #86007, it was noted that when collecting traces with verbose recording
enabled, a significant fraction of these traces were from low-level
database operations, particularly during the transaction run in
`execChangeReplicasTxn(..)`. This function performs necessary
bookkeeping of range descriptors when replica movement occurs, however
the high verbosity on the low-level database operations within the
transaction is not necessary when examining traces from higher-level
operations like replicate queue processing. Hence, this change
creates child tracing spans within `execChangeReplicasTxn` which
can be filtered out prior to logging the traces in the replicate queue
processing loop. This decreases the size of the logged trace by a factor
of 10, allowing the resulting log message to be much more succinct and
focused.

Release justification: Low-risk, high benefit observability change.
Release note: None

86779: ui: statement insight details page v1 r=ericharmeling a=ericharmeling

This PR adds the Statement Insights Details page to the DB Console.

https://www.loom.com/share/e9c7be3eb5da47b784b119b6fc9bbeb9

Release justification: low-risk updates to new functionality

Release note (ui change): Added statement insight details page to DB Console.

86829: sql: include regions information into the sampled query telemetry r=yuzefovich a=yuzefovich

**execstats: remove unused regions field**

This field was never actually populated nor used since it was introduced
in 9f716a7.

Release justification: low-risk cleanup.

Release note: None

**sql: include regions information into the sampled query telemetry**

This commit adds the regions (where SQL processors where planned for the
query) to the sampled query telemetry. This required a couple of minor
changes to derive the regions information stored in the instrumentation
helper earlier (before the logging takes place).

Fixes: #85427.

Release justification: low-risk improvement.

Release note (sql change): The structured payloads used for telemetry
logs now include the new `Regions` field which indicates the regions of
the nodes where SQL processing ran for the query.

86951: flowinfra: improve BenchmarkFlowSetup r=yuzefovich a=yuzefovich

This commit improves `BenchmarkFlowSetup` by pulling out parsing of the
statement from the hot path as well as more precise usage of the timer
(to exclude the construction of the internal planner). This way the
benchmark is better focused on its goal - the performance of the
planning and execution.

Release justification: test-only change.

Release note: None

86964: rowexec: close all ValueGenerators in the project set processor r=yuzefovich a=yuzefovich

Previously, we forgot to close the value generators when a new input row
is read by the project set processor which could lead to leaking of the
resources. This is now fixed. Most of the value generators don't need to
release any resources, a few need to close their memory account
(previously this would result in a sentry report, but no actual leak
would occur in production builds), the only concerning one is
`crdb_internal.payloads_for_trace` where we would not close the
`InternalRows` iterator. But that seems like an internal debugging tool,
so most likely the users weren't impacted.

Fixes: #85418.

Release justification: bug fix.

Release note: None (It seems like in most cases the users would not see
the impact.)

86965: pgcode: use custom error codes for home region errors r=msirek a=rafiss

The XC error class is reserved for Cockroach-specific errors to avoid
conflicting with any future error codes that Postgres might add.

Release note: None

Release justification: low risk change to new functionality.

Co-authored-by: j82w <jwilley@cockroachlabs.com>
Co-authored-by: Alex Sarkesian <sarkesian@cockroachlabs.com>
Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
@craig craig bot closed this as completed in 08f810f Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging a pull request may close this issue.

3 participants