From 99b7d1975854f07324722d3f372d77f0166aed93 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Tue, 18 Jul 2023 12:19:48 +0200 Subject: [PATCH] Bust Symbolicator `list_files` cache on uploads (#53049) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By attaching a random querystring parameter to the Sentry internal source, this will cause a cache miss in Symbolicators `list_files` cache, thus fetching and discovering the uploaded debug file immediately. This is very similar to `bump_reprocessing_revision`, except it has a TTL. Also the code for `bump_reprocessing_revision` is overly complex for reasons I don’t understand so I don’t want to touch that. --- src/sentry/lang/native/sources.py | 35 ++++++++++++++++++++++++++----- src/sentry/models/debugfile.py | 3 +++ src/sentry/tasks/assemble.py | 2 ++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/sentry/lang/native/sources.py b/src/sentry/lang/native/sources.py index 91e2de400b7a3..52a984bf6a5ef 100644 --- a/src/sentry/lang/native/sources.py +++ b/src/sentry/lang/native/sources.py @@ -2,6 +2,7 @@ import logging import sys from copy import deepcopy +from datetime import datetime import jsonschema import sentry_sdk @@ -10,7 +11,8 @@ from sentry import features, options from sentry.auth.system import get_system_token -from sentry.utils import json, safe +from sentry.models.project import Project +from sentry.utils import json, redis, safe logger = logging.getLogger(__name__) @@ -115,6 +117,22 @@ }, } +default_cluster = redis.redis_clusters.get("default") +LAST_UPLOAD_TTL = 24 * 3600 + + +def _last_upload_key(project_id: int) -> str: + return f"symbols:last_upload:{project_id}" + + +def record_last_upload(project: Project): + timestamp = int(datetime.utcnow().timestamp() * 1000) + default_cluster.setex(_last_upload_key(project.id), LAST_UPLOAD_TTL, timestamp) + + +def get_last_upload(project_id: int): + return default_cluster.get(_last_upload_key(project_id)) + class InvalidSourcesError(Exception): pass @@ -137,7 +155,7 @@ def get_internal_url_prefix() -> str: return internal_url_prefix.rstrip("/") -def get_internal_source(project): +def get_internal_source(project: Project): """ Returns the source configuration for a Sentry project. """ @@ -149,6 +167,13 @@ def get_internal_source(project): ), ) + if last_upload := get_last_upload(project.id): + # Adding a random query string parameter here makes sure that the + # Symbolicator-internal `list_files` cache that is querying this API + # is not being hit. This means that uploads will be immediately visible + # to Symbolicator, and not depending on its internal cache TTL. + sentry_source_url += f"?_last_upload={last_upload}" + return { "type": "sentry", "id": INTERNAL_SOURCE_NAME, @@ -157,7 +182,7 @@ def get_internal_source(project): } -def get_internal_artifact_lookup_source_url(project): +def get_internal_artifact_lookup_source_url(project: Project): """ Returns the url used as a part of source configuration for the Sentry artifact-lookup API. """ @@ -173,7 +198,7 @@ def get_internal_artifact_lookup_source_url(project): ) -def get_internal_artifact_lookup_source(project): +def get_internal_artifact_lookup_source(project: Project): """ Returns the source configuration for the Sentry artifact-lookup API. """ @@ -185,7 +210,7 @@ def get_internal_artifact_lookup_source(project): } -def is_internal_source_id(source_id): +def is_internal_source_id(source_id: str): """Determines if a DIF object source identifier is reserved for internal sentry use. This is trivial, but multiple functions in this file need to use the same definition. diff --git a/src/sentry/models/debugfile.py b/src/sentry/models/debugfile.py index f1fb902705ab1..a77cd70159a6c 100644 --- a/src/sentry/models/debugfile.py +++ b/src/sentry/models/debugfile.py @@ -598,6 +598,8 @@ def create_files_from_dif_zip( """Creates all missing debug files from the given zip file. This returns a list of all files created. """ + from sentry.lang.native.sources import record_last_upload + scratchpad = tempfile.mkdtemp() try: safe_extract_zip(fileobj, scratchpad, strip_toplevel=False) @@ -615,6 +617,7 @@ def create_files_from_dif_zip( rv = create_debug_file_from_dif(to_create, project) # Uploading new dsysm changes the reprocessing revision + record_last_upload(project) bump_reprocessing_revision(project) return rv diff --git a/src/sentry/tasks/assemble.py b/src/sentry/tasks/assemble.py index e797e7de1947e..4bcd9b0a8153f 100644 --- a/src/sentry/tasks/assemble.py +++ b/src/sentry/tasks/assemble.py @@ -185,6 +185,7 @@ def assemble_dif(project_id, name, checksum, chunks, debug_id=None, **kwargs): """ Assembles uploaded chunks into a ``ProjectDebugFile``. """ + from sentry.lang.native.sources import record_last_upload from sentry.models import BadDif, Project, debugfile from sentry.reprocessing import bump_reprocessing_revision @@ -239,6 +240,7 @@ def assemble_dif(project_id, name, checksum, chunks, debug_id=None, **kwargs): # and might resolve processing issues. If the file was not # created, someone else has created it and will bump the # revision instead. + record_last_upload(project) bump_reprocessing_revision(project, use_buffer=True) except Exception: set_assemble_status(