Skip to content

feat(taskworker): Update Taskbroker / Taskworker Client to Support Push Mode#112607

Closed
george-sentry wants to merge 314 commits intomasterfrom
george/feat/update-taskworker-to-push-mode
Closed

feat(taskworker): Update Taskbroker / Taskworker Client to Support Push Mode#112607
george-sentry wants to merge 314 commits intomasterfrom
george/feat/update-taskworker-to-push-mode

Conversation

@george-sentry
Copy link
Copy Markdown
Member

Linear

Completes STREAM-866

Description

Currently, taskworkers pull tasks from taskbrokers via RPC. This approach works, but has some drawbacks. Therefore, we want taskbrokers to push tasks to taskworkers instead. Read this page on Notion for more information.

This PR updates taskbroker-client package to version 0.1.8, which adds the ability to select between "pull" and "push" delivery modes. It also updates the run_taskworker command to accept extra arguments for configuring the taskworker in push mode...

  • --push-mode is a boolean that turns push mode on or off (default off)
  • --taskworker-rpc-port specifies the port the taskworker gRPC server should listen on (default is 50052 since the taskbroker runs on 50051, and in self or local hosted environments, brokers and workers may run on the same machine)

@george-sentry george-sentry requested review from a team as code owners April 9, 2026 18:52
@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 9, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 9, 2026
Comment thread src/sentry/runner/commands/run.py Outdated
Comment thread src/sentry/runner/commands/run.py Outdated
Comment thread src/sentry/runner/commands/run.py Outdated
Comment thread src/sentry/taskworker/adapters.py Outdated
Comment thread src/sentry/taskworker/adapters.py Outdated
Copy link
Copy Markdown
Member

@evanh evanh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

sample_rate=sample_rate,
unit=unit,
stacklevel=stacklevel,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent sample_rate handling in new gauge method

Low Severity

The new gauge method uses sample_rate: float = settings.SENTRY_METRICS_SAMPLE_RATE as its default, while every other method in SentryMetricsBackend (incr, distribution, timer) consistently uses sample_rate: float | None = None with a runtime if sample_rate is None: check. This means gauge evaluates the setting at class-load time (freezing the value), whereas all sibling methods resolve it at call time. If the abstract MetricsBackend from taskbroker_client passes None for sample_rate, this method won't handle it correctly.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 36cb982. Configure here.

TkDodo and others added 17 commits April 9, 2026 14:12
…s to apiOptions (#112347)

as discussed in last week’s TSC, we need to start moving endpoints over
to `apiOptions` so that we can re-use caches.

the quickest win would be to implement `useApiQuery` with `apiOptions`
internally, and to get there, we need to migrate endpoints that now need
`getResponseHeader` over to `apiOptions` because those headers are
exposed differently.

this PR takes the first couple of endpoints that are (mostly)
self-contained (no other usages of the url found) and moves them over.

I’ve also exposed `selectJsonWithHeaders` because the default impl only
selects `json` without `headers`.

I plan to tackle the other occurrences with an endpoint-by-endpoint
approach, as we need to identify all places where an endpoint is used
(e.g. `invalidateQueries` or `getApiQueryData` or `setApiQueryData`) and
migrate them together.
…okens (#111956)

To be able to do equations using series, we need to be able to support
UI that will allow us to reference other series definitions. To do this
I've added a `REFERENCE` token type which will be used in conjunction
with an argument passed in, `references`, which determines the available
references.

This required me to update the grammar, validators, and tokenizer to
have the right check for this "references" condition (i.e. if it's not
in the set, it's not a reference and it will fall back to free text)

The changes also follow closely to the code path for functions, so
places where it checks for functions as valid terms should also check
for references. These are used to update things like the autoformatting
and cursor placement when switching from freetext to a reference token.

I also changed some of the internals for how we handle
`static/app/components/arithmeticBuilder/action.tsx` because I want the
expression to be able to re-evaluate if the references change. If a
reference is removed, the equation should also update and render the old
reference as free text.

Since there's no UI using this at the moment, I've opted to render it in
Storybook
This PR refreshes the continue scanning logs experience to really call
out to users that they're able to continue scanning for more data, as
the previous setup was a bit harder to read.

Refs LOGS-644

---------

Co-authored-by: GPT-5.4 <noreply@openai.com>
Just adding in the content sections, no real functionality yet just
getting the layout set up.
Also added in an empty schema hints so i remember to fill it in along
with the search bar when possible💀

<img height="400" alt="image"
src="https://github.com/user-attachments/assets/0e8c1097-9f97-4399-9182-ecc456192b98"
/>
Remove dead profile insertion code (unused since
#102210; insertion now handled
by vroomrs) and the dead `_ProjectKeyKwargs` struct (unused since
#81957).
Register PREPROD in the shared `INTERNAL_TO_PUBLIC_ALIAS_MAPPINGS`
infrastructure so the EAP attributes endpoint translates
`has_installable_file` → `installable`. This makes the `installable`
boolean filter appear in the mobile builds search bar dropdown on the
Distribution view.

Previously, the PREPROD trace item type had no entry in the alias
mapping dicts in `search/eap/utils.py`, so
`translate_internal_to_public_alias()` returned the raw internal name
`has_installable_file`. The frontend `filterToAllowedKeys()` then
couldn't match it against the allowlist (which expects `installable`),
so the attribute was silently filtered out.

The mapping dict is built dynamically from
`PREPROD_SIZE_ATTRIBUTE_DEFINITIONS`, so any future aliased PREPROD
attributes will automatically be included.

Refs EME-868

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…tings (#112232)

This brings in the stopping-point dropdown to the project-specific seer
settings page

Follows: #112211

There's still more to cleanup in here, but we're well on the way.

<img width="960" height="383" alt="SCR-20260403-msjw"
src="https://github.com/user-attachments/assets/09b86dd8-6a0e-4bf5-89d9-9cd9d9649f6d"
/>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…bring back Create PR bulk edits (#112249)

<img width="279" height="198" alt="SCR-20260403-oruo"
src="https://github.com/user-attachments/assets/c8b49017-40fe-49ec-aa47-d364ed9b4d8c"
/>

---------

Co-authored-by: Claude Sonnet 4 <noreply@example.com>
We already have logging for `data-export-success`. That success log
includes the user's email, which makes it nice and easy to tell if
someone's data export has succeeded (at least this far in code). But
there's no equivalent log for failure. Which is what @narsaynorath and I
were trying to debug earlier today and having to do all sorts of
log/trace query workarounds for.

Fixes LOGS-673.
Extracts a `formatNumber` helper for the numeric field renderer. It
bypasses `formatFloat` for numbers that have over 13 digits.

Tangentially related:

* #110360
* #110858

Fixes EXP-866.

Co-authored-by: Cursor <noreply@cursor.com>
<!-- Describe your PR here. -->

<!--

  Sentry employees and contractors can delete or ignore the following.

-->

### Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.
#109819)

Previously the `LogsInfiniteTable` component generally took in a
`scrollContainer` ref to bind its scrolling to its parent (or, by
default, the window). Its internal virtualizer then switched between the
container vs. window based on whether that was available.

This PR simplifies the whole situation by having the table manage its
own height with an `expanded` piece of state & an "expando" button to
toggle between collapsed & expanded. The expando button is hidden when
the table has the `embedded` prop.

The functional changes are gated behind an
`organizations:ourlogs-table-expando` feature flag but can also be
enabled with a `?logsTableExpando=true` query parameter. I couldn't
reasonably gate all of the far-reaching layout changes behind that, but
do think I got the experience equivalent in the default/off state.


<table>
<thead>
<tr>
<th>Area & URL</th>
<th>Before</th>
<th>After</th>
</tr>
</thead>
<tbody>
<tr>
<td>
  Issue Logs Drawer
  <br />
  <code>/issues/*</code>
</td>
<td>
<img width="1645" height="965" alt="image"
src="https://github.com/user-attachments/assets/f937ca71-2da7-4ed3-9e16-2abfecf7619d"
/>
</td>
<td>
<em>(same)</em>
</td>
</tr>
<tr>
<td>
  Explore > Logs
  <br />
  <code>/explore/logs</code>
</td>
<td>
<img width="1249" height="862" alt="image"
src="https://github.com/user-attachments/assets/0e069571-7834-4071-9b09-570c30a87646"
/>
</td>
<td>
<img width="1249" height="862" alt="image"
src="https://github.com/user-attachments/assets/7c2caf28-f0b0-402d-956e-9ee5bf6f927d"
/>
<img width="1249" height="862" alt="image"
src="https://github.com/user-attachments/assets/f3dff911-e7d3-488f-a3af-bf9b48d26683"
/>
</td>
</tr>
<tr>
<td>
  Trace > Logs
  <br />
  <code>/explore/traces/trace/*/?&tab=logs</code>
</td>
<td>
<img width="1655" height="965" alt="image"
src="https://github.com/user-attachments/assets/b973c463-3234-45b6-8ee7-ca53cb4c8037"
/>
</td>
<td>
<em>(same)</em>
</td>
</tr>
<tr>
<td>
  Trace > Log Details
  <br />
  <code>/explore/metrics/trace/*</code>
</td>
<td>
<img width="1844" height="963" alt="image"
src="https://github.com/user-attachments/assets/17a1a7e3-14f3-4f9f-9c32-e56c8098a952"
/>
</td>
<td>
<em>(same)</em>
</td>
</tr>
<tr>
<td>
  Replay > Logs
  <br />
  <code>/explore/replays/*/?&t_main=logs</code>
</td>
<td>
<img width="1655" height="965" alt="image"
src="https://github.com/user-attachments/assets/a8b70862-d9ec-43b3-ab9e-b5439d457d0e"
/>
</td>
<td>
<em>(same)</em>
</td>
</tr>
</tbody>
</table>

A few unit tests around log UIs were failing because JSDom's default
zero-sized containers result in zero rows being rendered. This PR adds a
shared `mockGetBoundingClientRect` for them.

There is one known bug: [LOGS-653 Logs table virtualization gets offset
after expanding & contracting
rows](https://linear.app/getsentry/issue/LOGS-653/logs-table-virtualization-gets-offset-after-expanding-and-contracting).
It's an edge case that doesn't always show up, so we think this is only
a blocker for releasing to prod, not releasing internally.

Fixes LOGS-173. Fixes LOGS-591.

---------

Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
remove the `organizations:slack-compact-alerts` feature flag and make
the compact slack alert layout the default for all users including
self-hosted. removes dead code paths for the non-compact layout
(headline formatting, suspect commits block, separate suggested
assignees block, trailing divider).

Fixes ISWF-2375

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This adds a task that runs every 24 hours to make sure that the
available repositories on the github side match the repositories we have
on the Sentry side.
The mobile-builds settings page now uses tabs, so the "Configure status
check rules" link in VCS size status checks needs to include `tab=size`
to land on the correct tab instead of the default.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
… filter (#112368)

Handle null provider in the organization integrations filter on the
notification settings page.

The `/users/me/organization-integrations/` API can return entries with a
null `provider` field. The `ALLOWED_PROVIDERS` filter added in #111745
accesses `orgIntegration.provider.key` without null safety, crashing the
page with `TypeError: Cannot read properties of null (reading
'provider')`.

Adds optional chaining to match the pattern already used for the
identity
provider filter a few lines above.

handles JAVASCRIPT-38j8 & JAVASCRIPT-38N4

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@george-sentry george-sentry requested review from a team as code owners April 9, 2026 21:33
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 9, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

@sentry
Copy link
Copy Markdown
Contributor

sentry bot commented Apr 9, 2026

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 13096878

@george-sentry
Copy link
Copy Markdown
Member Author

I have no idea what just happened, sorry 🙃

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ceedb2c. Configure here.


log(f"Snuba bootstrap complete ({time.monotonic() - start:.0f}s total)")
SNUBA_EXIT.write_text(str(rc))
sys.exit(rc)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unhandled exceptions skip exit marker, causing CI timeout

Medium Severity

The main() function in bootstrap-snuba.py has no top-level exception handler. If an unhandled exception occurs before SNUBA_EXIT.write_text(str(rc)) runs (e.g., int(workers_str) raising ValueError for a non-numeric XDIST_WORKERS, or ThreadPoolExecutor(max_workers=0) if workers is zero), the /tmp/snuba-bootstrap-exit marker file is never written. The downstream "Wait for Snuba bootstrap" step polls this file in a loop and would silently hang for 600 seconds before timing out, making the root cause very hard to diagnose.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ceedb2c. Configure here.

if isinstance(target, ast.Name):
scope[target.id] = node.value
elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name) and node.value:
scope[node.target.id] = node.value
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AST scope misses class-level parametrize variable references

Low Severity

The scope dict is built only from ast.iter_child_nodes(tree), which yields module-level assignments. Variables defined inside class bodies (e.g., CASES = [...] on a test class) are never captured. When ast.walk later visits parametrized methods on those classes, the variable lookup in _resolve fails and the decorator silently falls back to a count of 1 instead of the actual parameter count. This causes systematic undercounting of tests using class-level parametrize variables.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ceedb2c. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.