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

cluster-ui: fix app filtering and selection #70998

Closed
xinhaoz opened this issue Oct 1, 2021 · 1 comment · Fixed by #70999
Closed

cluster-ui: fix app filtering and selection #70998

xinhaoz opened this issue Oct 1, 2021 · 1 comment · Fixed by #70999
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. GA-blocker

Comments

@xinhaoz
Copy link
Member

xinhaoz commented Oct 1, 2021

Describe the problem

On statements page, the app name should be derived from the route params.

In #70600, the selection of the app name was mistakenly switched from a route param attribute to a query string parameter. The correct behaviour is to get the app name from the route param.

To Reproduce

On CC:

  1. Try to filter by app name, or select any other filter and observe that the app name is not correctly passed to the statement details pages, since the selection of the app name is incorrect.
    Expected behavior
    App name is selected correctly based on the route param.
@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 GA-blocker backport-21.2.x labels Oct 1, 2021
@blathers-crl
Copy link

blathers-crl bot commented Oct 1, 2021

Hi @xinhaoz, please add branch-* labels to identify which branch(es) this release-blocker affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Oct 1, 2021
@xinhaoz
cluster-ui: derive app name from route parameter in cluster-ui
a6f99c0 
Fixes: cockroachdb#70998

Release justification: category 2

Previously, we were deriving the selected app name from the
query string parameter in for the statements page in the
cluster-ui package. The selected app name should be derived from
the route parameter for the statements page.

Release note (bug fix): the selected app name in the statements page
is now derived from the route parameters.
@Azhng Azhng mentioned this issue Oct 1, 2021
Merged
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Oct 1, 2021
@xinhaoz
cluster-ui: derive app name from route parameter in cluster-ui
3dd506d 
Fixes: cockroachdb#70998

Release justification: category 2

Previously, we were deriving the selected app name from the
query string parameter in for the statements page in the
cluster-ui package. The selected app name should be derived from
the route parameter for the statements page.

Release note (bug fix): the selected app name in the statements page
is now derived from the route parameters.
craig bot pushed a commit that referenced this issue Oct 1, 2021
@celiala @andreimatei @xinhaoz
Merge #69904 #70850 #70999
3426e8d 
69904: licenses: updating licenses r=celiala a=celiala

As part of [Creating a release branch](https://cockroachlabs.atlassian.net/wiki/spaces/ENG/pages/187859111/Creating+a+release+branch) for release-21.2, this PR:
- Adds the following new licenses:
	- BSL-1.0 (Boost Software License)
	- CC-BY-3.0
	- CC0-1.0
	- JSON
	- MPL-2.0
	- The Unlicense
- Updates the BSL change date for 21.2 (we missed updating this last Spring; a separate PR will update this for master/22.1 after branch cut).

This PR is one of the tasks detailed in #70751, which tracks all the steps relevant to creating a release branch for `${vBRANCH_CUT}` and preparing master for the `${vNEXT}` major version.

Release justification: Non-production code change
Release note: None

70850: util/tracing: fix an edge case for active span registration r=andreimatei a=andreimatei

Before this patch, a span with a no-op parent (as opposed to a span
with no parent), would not be present in the active spans registry. The
code was confused: the span didn't qualify as a "local root" because it
had a local parent, but of course it also wasn't really recorded by the
local parent cause a no-op span can't record anything. As such, the span
in question was missing from the registry.
This patch makes a span with a no-op parent behave like a root span.

Release note: None

70999: cluster-ui: derive app name from route parameter in cluster-ui r=maryliag a=xinhaoz

Fixes: #70998

Release justification: category 2

Previously, we were deriving the selected app name from the
query string parameter in for the statements page in the
cluster-ui package. The selected app name should be derived from
the route parameter for the statements page.

Release note (bug fix): the selected app name in the statements page
is now derived from the route parameters.

Co-authored-by: Celia La <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
@craig craig bot closed this as completed in cb31d8d Oct 1, 2021
blathers-crl bot pushed a commit that referenced this issue Oct 1, 2021
@xinhaoz
cluster-ui: derive app name from route parameter in cluster-ui
ac4ade9 
Fixes: #70998

Release justification: category 2

Previously, we were deriving the selected app name from the
query string parameter in for the statements page in the
cluster-ui package. The selected app name should be derived from
the route parameter for the statements page.

Release note (bug fix): the selected app name in the statements page
is now derived from the route parameters.
@blathers-crl blathers-crl bot mentioned this issue Oct 1, 2021
Merged
andy-kimball pushed a commit that referenced this issue Oct 6, 2021
@xinhaoz @andy-kimball
cluster-ui: derive app name from route parameter in cluster-ui
ee5cf00 
Fixes: #70998

Release justification: category 2

Previously, we were deriving the selected app name from the
query string parameter in for the statements page in the
cluster-ui package. The selected app name should be derived from
the route parameter for the statements page.

Release note (bug fix): the selected app name in the statements page
is now derived from the route parameters.
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. GA-blocker
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cluster-ui: derive app name from route parameter in cluster-ui xinhaoz/cockroach
1 participant
@xinhaoz