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

Build v2 of tracez page #83679

Closed
benbardin opened this issue Jun 30, 2022 · 1 comment
Closed

Build v2 of tracez page #83679

benbardin opened this issue Jun 30, 2022 · 1 comment
Assignees
Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-disaster-recovery

Comments

@benbardin
Copy link
Collaborator

benbardin commented Jun 30, 2022

The current page has the information we need, but it's not always well-organized. We want to improve this page to enhance observability and improve debugging of unexpected behavior.

Jira issue: CRDB-17204

Epic CRDB-19704

@benbardin benbardin added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-disaster-recovery T-disaster-recovery labels Jun 30, 2022
@blathers-crl
Copy link

blathers-crl bot commented Jun 30, 2022

cc @cockroachdb/bulk-io

craig bot pushed a commit that referenced this issue Oct 13, 2022
88181: pkg/ui: add first stage of redesigned tracez page r=benbardin a=benbardin

Begins to implement

https://www.figma.com/file/LUptGAxt5x1idq6eZLA50o/BulkIO-Traces?node-id=59%3A7958

When complete, the new pages will offer:
- A robust route structure and full redux integration for better
navigation.
- Denser information display.
- More information display, i.e. start time/duration, and expanding mutliple
tag groups.
- A new span details page, which will include child span information.

For now, this page is behind a new route with no entrypoint.
It implements the core route structure, redux selectors and
API calls.
Left for later PRs are Take Snapshot, the Span Details page, pagination,
search filtering, and View Raw JSON.

Part of #83679

Release note: None

![Screen Shot 2022-10-04 at 12 46 07 PM](https://user-images.githubusercontent.com/261508/193897216-dd6f811e-c6da-4916-8f2f-a8042040e6ba.png)
![Screen Shot 2022-10-04 at 12 45 53 PM](https://user-images.githubusercontent.com/261508/193897217-61f41cb3-1341-4f42-be31-07035ccf6dea.png)
![Screen Shot 2022-10-04 at 12 45 37 PM](https://user-images.githubusercontent.com/261508/193897218-975bf6ec-d5ae-4a1f-9522-494500426b1e.png)
![Screen Shot 2022-10-04 at 12 45 27 PM](https://user-images.githubusercontent.com/261508/193897220-0c0b5028-1f00-4d90-afed-2697f9d47056.png)
![Screen Shot 2022-10-04 at 12 45 17 PM](https://user-images.githubusercontent.com/261508/193897223-c6f92740-67af-4b1e-a69e-5f536d9759c8.png)



89848: sql: fix trigram span generation for similarity filters r=mgartner a=mgartner

This commit fixes a bug in the generation of trigram inverted index spans for similarity filters. The bug could cause rows to be incorrectly filtered out of results.

Previously, we did not generate padded trigrams when building the inverted spans for similarity filters. Now, we generate padded trigrams to correct the bug.

For example, for a filter such as `col % 'aab'`, we would generate the single trigram `'aab'` and the corresponding span `['aab'-'aab']`. This span does not contain all indexed trigrams of values that are similar to `'aab'`. As an example, it covers none of the trigrams of `'aaaaaa'`, which are `{'  a',' aa','aa ','aaa'}`. Now, for the same expression `col % 'aab'`, we generate the padded trigrams `{'  a',' aa','aab','ab '}` and the corresponding spans `['  a'-'  a'], [' aa'-' aa'], ['aab'-'aab'], ['ab '-'ab ']` which contain some of the trigrams of `'aaaaaa'`.

Fixes #89609

Release note (bug fix): A bug has been fixed that caused incorrect results for queries with string similar filters (e.g., `col % 'abc'`) on tables with trigram indexes. This bug is only present in 22.2 pre-release versions up to and including v22.2.0-beta.3.

89924: ci: let testrace use `simplestamp` config r=rickystewart a=healthy-pod

Release note: None
Epic: none

Co-authored-by: Ben Bardin <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: healthy-pod <[email protected]>
craig bot pushed a commit that referenced this issue Oct 14, 2022
89929: storage: delete deprecated multiIterator r=erikgrinaker a=msbutler

Use the new NewPebbleSSTIterator() instead.

Fixes #87943

Release note: None

89979: pkg/ui: performance fixes on redesigned tracez page r=benbardin a=benbardin

Followup to #88181 

Part of #83679

Release note: None

Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: Ben Bardin <[email protected]>
craig bot pushed a commit that referenced this issue Nov 16, 2022
90244: pkg/ui: Make tracez v2 page node-aware r=benbardin a=benbardin

Release note: None

Demo video below. Note that it starts on the v1 trace page, and I navigate to the new one by typing in the address bar.

https://user-images.githubusercontent.com/261508/196736069-952dadde-dcb7-4060-8563-e98727b48a48.mov

Part of #83679


92007: ui: tidy some SQL Activity code r=matthewtodd a=matthewtodd

Cleanup encountered along the way to adding region columns & filters for serverless.

Simplest to review commit by commit. The changes are small, but the commit message notes help add context.

Part of #89949

Co-authored-by: Ben Bardin <[email protected]>
Co-authored-by: Matthew Todd <[email protected]>
craig bot pushed a commit that referenced this issue Jan 3, 2023
93207: pkg/ui: Add span details component to tracez_v2. r=benbardin a=benbardin

This PR adds the "span details" and "raw trace" screens to the new tracez_v2 page. Previously, only the "snapshot details (i.e. list spans) screen existed on that page. The detailed span information page is where users will go to dig in to the specifics of a span. In particular, we'll display accumulated data on child spans of that page, helping navigate to problem segments more easily.

Note that this PR brings the v2 page up to feature parity with the v1 page. Though the redesign is not yet complete, we'll be able to turn down the v1 page after this lands.

This PR also modifies the route structure a little to encapsulate better.

Release note: None

<img width="1583" alt="Screen Shot 2022-12-07 at 1 52 26 PM" src="https://user-images.githubusercontent.com/261508/206270214-9708e298-cc8e-4a69-9554-8bfb8555277c.png">

<img width="1583" alt="Screen Shot 2022-12-07 at 1 51 59 PM" src="https://user-images.githubusercontent.com/261508/206270152-8d0a36f0-c64d-40bd-a47e-1573039e33ac.png">

<img width="1664" alt="Screen Shot 2022-12-05 at 12 12 41 PM" src="https://user-images.githubusercontent.com/261508/206269949-a62130c7-53cf-4655-84ae-8f6c42d4e66e.png">

Informs #83679

94661: coldata: fix recent flaky behavior r=yuzefovich a=yuzefovich

In a just merged b353d18 when addressing the PR feedback I introduced a possibility of a crash in tests. In particular, this can occur if we choose to randomize `coldata.BatchSize` value larger than the default (or if the test explicitly overrides it). The problem is that `BatchSize()` can return different values in `init()` of `coldata` package and during the test runs, so we now use `MaxBatchSize` where applicable.

Epic: None

Release note: None

Co-authored-by: Ben Bardin <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
benbardin added a commit to benbardin/cockroach that referenced this issue Jan 4, 2023
benbardin added a commit to benbardin/cockroach that referenced this issue Jan 11, 2023
craig bot pushed a commit that referenced this issue Jan 11, 2023
94701: pkg/ui: Swap in new Snapshot debug component r=benbardin a=benbardin

The PR focuses on the routes. I'll delete deprecated code in a follow-up diff.

Part of: #83679

Release note: None

Co-authored-by: Ben Bardin <[email protected]>
craig bot pushed a commit that referenced this issue Jan 17, 2023
93301: sql,backupccl: set system.role_members ID columns to NOT NULL r=stevendanna,rafiss a=andyyang890

This patch sets the newly added `role_id` and `member_id` columns in
`system.role_members` to be NOT NULL. It also changes the `RESTORE`
logic to be able to handle the case when restoring from a backup where
the `system.role_members` table did not have the columns.

Part of #87079

Release note: None

94796: changefeedccl: add new fine-grained permissions  r=jayshrivastava a=jayshrivastava

This change updates permissions semantics related to creating and managing changefeeds.

### Summary
The `CONTROLCHANGEFEED` role option will be deprecated in the future (see #94757). With this change, usages of `CONTROLCHANGEFEED` will come with a deprecation warning. Its existing behavior (see rules for creating changefeeds below) remains the same.

The `SELECT` and `CHANGEFEED` privileges will be used for changefeeds henceforth:

The `SELECT` privilege on a set of tables allows a user to run core changefeeds against them.

The `CHANGEFEED` privilege on a set of tables allows a user to run enterprise changefeeds on them, and also manage the underlying changefeed job (ie. view, pause, cancel, and resume the job). Notably, a new cluster setting `changefeed.permissions.enforce_external_connections` is added and set to `false` by default. Enabling this setting restricts users with `CHANGEFEED` on a set of tables to create enterprise changefeeds into external connections only. To use a given external connection, a user typically needs the `USAGE` privilege on it.

Note `ALTER DEFAULT PRIVILEGES` can be used with both the `CHANGEFEED` and `SELECT` privileges to assign course-grained permissions (ie. assign permissions to all tables in a schema rather than manually assign them for each table).

### Additional Details

#### Creating a Changefeed

Before this change, to create a changefeed, these checks were made in order:
(a) If the user has the `CONTROLCHANGEFEED` role, then they require the `SELECT`privilege 
     on all targeted tables
(b) Otherwise, the user requires  the `CHANGEFEED` privilege on all targeted tables
With this change, these checks are updated:
(a) If the user has the `CONTROLCHANGEFEED` role, then they require the `SELECT` 
     privilege on all targeted tables. Note: creating a changefeed this way will now produce a 
     deprecation notice.
(b) If the changefeed is a core changefeed, they require the `SELECT` privilege on all targeted 
     tables
(c) Otherwise, the user requires the `CHANGEFEED` privilege on all targeted tables. Note: If
     `changefeed.permissions.enforce_external_connections` (disabled by default) is set to true,
     then the user will only be able to create a changefeed into an external connection which they
     have the `USAGE` privilege on.

#### Managing a Changefeed

Before this change, to manage a changefeed job `J` (defined a viewing, pausing, resuming, and canceling), a user `U` could do so if they met at least one of the following conditions:
(a) `U` is an admin
(b) `U` is not an admin and `J` is owned by `U`  (only for SHOW JOBS)
(c) `U` is not an admin, `J` is not owned by an admin, and `U` has the `CONTROLJOB` role
With this change, the conditions are updated:
(a) `U` is an admin
(b) `U` is not an admin and `J` is owned by `U`  (only for `SHOW JOBS` or `SHOW 
     CHANGEFEED JOBS`)
(c) `U` is not an admin, `J` is not owned by an admin, and `U` has the `CONTROLJOB` role
(d) `U` is not an admin, `J` is not owned by an admin, `J` is a changefeed job, and `U` has 
     the `CHANGEFEED` privilege on targeted tables

#### Altering a Changefeed

Before this change, permissions related to altering changefeeds with `ALTER CHANGEFEED` were not explicitly defined (we did not have tests to assert its behavior, but there were some permissions checks regardless). Basically, a user needed access to view a job (ie. look up it’s job ID via `SHOW JOBS`) and they needed to be able to create a new job. After all, `ALTER CHANGEFEED` is essentially the same as creating a new job after stopping the old one.

With this change, the same rules apply: the user needs to be able to access the existing job and to be able to create a new changefeed with the new rules introduced in this change respectively.

Fixes: #94756
Fixes: #92261
Fixes: #87884
Fixes: #85082
Fixes: #94759
Informs: #94757
Epic: CRDB-19709

Release note (enterprise change):

The `CONTROLCHANGEFEED` role option will be deprecated in the future (see #94757). With this change, usages of `CONTROLCHANGEFEED` will come with a deprecation warning. Its existing behavior (see rules for creating changefeeds above) remains the same.

The `SELECT` and `CHANGEFEED` privileges will be used for changefeeds henceforth:

The `SELECT` privilege on a set of tables allows a user to run core changefeeds against them.

The `CHANGEFEED` privilege on a set of tables allows a user to run enterprise changefeeds on them, and also manage the underlying changefeed job (ie. view, pause, cancel, and resume the job). Notably, a new cluster setting `changefeed.permissions.enforce_external_connections` is added and set to `false` by default. Enabling this setting restricts users with `CHANGEFEED` on a set of tables to create enterprise changefeeds into external connections only. To use a given external connection, a user typically needs the `USAGE` privilege on it.

Note `ALTER DEFAULT PRIVILEGES` can be used with both both the `CHANGEFEED` and `SELECT` privileges to assign coarse-grained permissions (ie. assign permissions to all tables in a schema rather than manually assign them for each table).




94968: server: tenant access to node diagnostics and metrics r=abarganier,knz a=dhartunian

Previously, tenants were unable to access cluster-level node information, and could not make timeseries queries. This is due to the fact that tenants do not have access to gossip data and cannot make aritrary KV requests. Any of these requests must go through the `kvtenant.Connector` interface which authorizes these requests and proxies them over gRPC to the KV server.

Future changes will gate these abilities behind Tenant privileges.

Epic: CRDB-12100

Resolves: #94967

Release note: None

95168: pkg/ui: Remove deprecated tracez frontend code. r=benbardin a=benbardin

Release note: None
Part of: #83679

95327: grunning: synchronize access to results in `TestProportionalGoroutines` r=rickystewart a=healthy-pod

This code change fixes a race in `TestProportionalGoroutines`.

Release note: None
Epic: none

Co-authored-by: Andy Yang <[email protected]>
Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: David Hartunian <[email protected]>
Co-authored-by: Ben Bardin <[email protected]>
Co-authored-by: healthy-pod <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-disaster-recovery
Projects
No open projects
Archived in project
Development

No branches or pull requests

1 participant