Skip to content

fix(relay): Refresh project and organization options before computing a config - #127930

Open
marius-mbravio wants to merge 1 commit into
getsentry:masterfrom
marius-mbravio:fix/relay-task-local-project-options
Open

marius-mbravio wants to merge 1 commit into
getsentry:masterfrom
marius-mbravio:fix/relay-task-local-project-options

Conversation

@marius-mbravio

Copy link
Copy Markdown

Organization and project option changes, such as new data scrubbing rules, don't reach Relay when a long-running taskworker rebuilds project configs. The worker keeps the options it read first in its process-local cache. request_finished clears that cache for web requests, but nothing clears it between tasks.

Fixes #127554

Cause. compute_projectkey_config already calls OrganizationOption.objects.reload_task_local_cache (#117602), but that method only drops the local copy when the shared cache is empty. Saving an option refills the shared cache with the new values, so the stale local copy is kept. Project options have no refresh at all, and relay/datascrubbing.py merges them with the organization's (sentry:relay_pii_config, sentry:sensitive_fields, sentry:scrub_data and others), so project-level scrubbing changes are delayed the same way.

Change.

  • reload_task_local_cache moves to OptionManager. It copies the shared cache into the local one when the shared cache is populated, and drops the local copy otherwise, so the next read queries the database.
  • compute_projectkey_config calls it for the project as well as the organization.

Tests.

  • Organization and project options: a value saved by another process (database and shared cache updated, this process's local cache stale) is picked up after the reload. With no shared cache, the reload falls back to the database.
  • compute_projectkey_config returns the organization and project sensitive_fields saved by another process.
  • On master, the organization reload test, both project tests and the config test fail. Removing either half of the change on its own turns at least one of them red.

Relation to #127929. That draft fixes the organization side and notes the project side as a possible follow-up. We filed #127554 and need project-level settings fixed too, so this change covers both. Whether it supersedes #127929, or the project half is folded into it, is up to you. Thanks to ranmori for the analysis of the organization side.

We reproduced the bug on self-hosted 26.7.2; 26.9.0 has the same code.

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.

… a config

A long-running taskworker keeps the organization and project options it read
first in its process-local option cache. `request_finished` clears that cache
for web requests, but nothing clears it between tasks, so configs computed in
the worker kept using settings saved after the worker first read them, such as
new data scrubbing rules.

`compute_projectkey_config` already called
`OrganizationOption.objects.reload_task_local_cache`, but that only dropped the
local copy when the shared cache was empty. Saving an option refills the shared
cache with the new values, so the stale local copy was kept. Project options had
no refresh at all.

`reload_task_local_cache` now lives on `OptionManager`: it copies the shared
cache into the local one when it is populated, and drops the local copy
otherwise. `compute_projectkey_config` calls it for the project as well as the
organization.

Fixes getsentry#127554
@marius-mbravio

Copy link
Copy Markdown
Author

The root cause turned out to be wider than the Relay path: the per-task cache clear from Celery's task_postrun was lost in the move to taskworker. #127932 restores it as a taskworker ContextHook. This PR stays as the targeted fix for project configs; with the hook in place it becomes redundant for new tasks but still covers a long task that builds many configs.

This branch has not been deployed

No deployments
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data scrubbing changes never reach Relay: taskworker rebuilds project configs from stale organization options

1 participant