Repository navigation
fix(relay): Refresh project and organization options before computing a config - #127930
Open
marius-mbravio wants to merge 1 commit into
Open
marius-mbravio wants to merge 1 commit into
marius-mbravio wants to merge 1 commit into
Conversation
… 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
Author
|
The root cause turned out to be wider than the Relay path: the per-task cache clear from Celery's |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_finishedclears that cache for web requests, but nothing clears it between tasks.Fixes #127554
Cause.
compute_projectkey_configalready callsOrganizationOption.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, andrelay/datascrubbing.pymerges them with the organization's (sentry:relay_pii_config,sentry:sensitive_fields,sentry:scrub_dataand others), so project-level scrubbing changes are delayed the same way.Change.
reload_task_local_cachemoves toOptionManager. 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_configcalls it for the project as well as the organization.Tests.
compute_projectkey_configreturns the organization and projectsensitive_fieldssaved by another process.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.