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.

Sign up for GitHub

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

Jump to bottom

ui: statement details page incorrectly sets appNames as (unset) in the stmt details request #83002

Closed
xinhaoz opened this issue Jun 16, 2022 · 0 comments · Fixed by #83008
Closed

ui: statement details page incorrectly sets appNames as (unset) in the stmt details request #83002

xinhaoz opened this issue Jun 16, 2022 · 0 comments · Fixed by #83008
Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@xinhaoz
Copy link
Member

xinhaoz commented Jun 16, 2022 •
edited by cockroach-jira-scripts
Loading

The statement details page does not properly handle the unset application name.
Currently, the page sets appNames in the request for statement fingerprint details as (unset) when it should be empty string, since we use (unset) in the ui to signal the application name as the empty string. This leads to the server not being able to find the data for the requested statement fingerprint id.

image

Jira issue: CRDB-16792
@xinhaoz xinhaoz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-observability Related to observability of the SQL layer T-sql-observability labels Jun 16, 2022
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 16, 2022
@maryliag
server: proper handle of unset app name
07e1d97 
Previously, if a filter for `unset` app name
was selected on Statement page, the Stament Details
page was not finding the correct values.
This commit proper handles with the unset value on
filter.

Fixes cockroachdb#83002

Release note (bug fix): Statement Details now find the stats
when the `unset` app filter is selected.
@maryliag maryliag mentioned this issue Jun 16, 2022
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 16, 2022
@maryliag
ui: proper handle of unset app name
6e43086 
Previously, if a filter for `unset` app name
was selected on Statement page, the Stament Details
page was not finding the correct values.
Now if no filter is selected, no search params is
passed to the Details page. If there is an empty value
for the appNames on request, is looking for unset.

This commit also creates a constant for the unset value
and replaces all places to use the constant instead.

Fixes cockroachdb#83002

Release note (bug fix): Statement Details now find the stats
when the `unset` app filter is selected.
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 22, 2022
@maryliag
ui: proper handle of unset app name
5867d63 
Previously, if a filter for `unset` app name
was selected on Statement page, the Stament Details
page was not finding the correct values.
Now if no filter is selected, no search params is
passed to the Details page. If there is an empty value
for the appNames on request, is looking for unset.

This commit also creates a constant for the unset value
and replaces all places to use the constant instead.

Fixes cockroachdb#83002

Release note (bug fix): Statement Details now find the stats
when the `unset` app filter is selected.
craig bot pushed a commit that referenced this issue Jun 24, 2022
@rafiss @knz
@maryliag @andreimatei @yuzefovich
Merge #82938 #82992 #83008 #83290 #83299
7af6c0d 
82938: roachtest: new passing tests for ruby-pg r=rafiss a=rafiss

fixes #82782

Release note: None

82992: roachtest,CODEOWNERS: remove primary assignments to the server team r=celiala a=knz

As discussed with `@lunevalex` and `@jtsiros` .

As the server team is currently unstaffed, we don't want issues to
be automatically assigned to it. This commit redirects the
issues to either the appropriate team, or to the virtual "noowner"
team.

The `cockroach/server-prs` notification target remains in CODEOWNERS
so that folk can use it to register to changes to particular code areas.

Release note: None

83008: ui: proper handle of unset app name r=maryliag a=maryliag

Previously, if a filter for `unset` app name
was selected on Statement page, the Statement Details
page was not finding the correct values.
Now if no filter is selected, no search params is
passed to the Details page. If there is an empty value
for the appNames on request, is looking for unset.

This commit also creates a constant for the unset value
and replaces all places to use the constant instead.

Fixes #83002

https://www.loom.com/share/2bcb83ef35714fc4b90be8d513017eab

Release note (bug fix): Statement Details now find the stats
when the `unset` app filter is selected.

83290: util/stop: fix span use when recovering from a panic r=andreimatei a=andreimatei

When recovering from a panic, the stopper was first finishing the
tracing span and then using it to log the panic. This was a
use-after-finish. It was not caught by the unit test because the test
was short-circuiting the logging part; I've improved the test so that it
would have caught it.

Release note: None

83299: sql: harden the check for when leaf txn can used r=yuzefovich a=yuzefovich

This commit refactors the code where we determine whether we have to
make a leaf txn for the local flow (which is the case when we either
parallelize scans or use the streamer API) to be "stronger". We recently
saw a nil-pointer error when trying to create a streamer (the txn was
nil), and the only case I could see this happening was that if
`makeLeaf` function returned nil which can occur if the original txn was
nil. I still don't see how that could happen (and neither could
I reproduce it), but this commit makes the code more bullet-proof so
that we ensure that `LeafTxnInputState` is non-nil when we expect to
have concurrency.

Fixes: #83259.

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
@craig craig bot closed this as completed in e16c7be Jun 24, 2022
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 24, 2022
@maryliag
ui: proper handle of unset app name
d8954c5 
Previously, if a filter for `unset` app name
was selected on Statement page, the Stament Details
page was not finding the correct values.
Now if no filter is selected, no search params is
passed to the Details page. If there is an empty value
for the appNames on request, is looking for unset.

This commit also creates a constant for the unset value
and replaces all places to use the constant instead.

Fixes cockroachdb#83002

Release note (bug fix): Statement Details now find the stats
when the `unset` app filter is selected.
@maryliag maryliag mentioned this issue Jun 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ui: proper handle of unset app name maryliag/cockroach
1 participant
@xinhaoz