Skip to content

Commit

Permalink
feat(proguard): A/B test new Symbolicator implementation (#67784)
Browse files Browse the repository at this point in the history
This adds a new rate option `"symbolicator.proguard-processing-ab-test"`
that causes proguard events to be processed using Symbolicator in
addition to the existing Python method and the results to be diffed.
  • Loading branch information
loewenheim committed Apr 3, 2024
1 parent 3a38896 commit b34eaf0
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 9 deletions.
61 changes: 61 additions & 0 deletions src/sentry/lang/java/plugin.py
Expand Up @@ -15,6 +15,7 @@
from sentry.plugins.base.v2 import Plugin2
from sentry.reprocessing import report_processing_issue
from sentry.stacktraces.processing import StacktraceProcessor
from sentry.utils.safe import get_path


class JavaStacktraceProcessor(StacktraceProcessor):
Expand Down Expand Up @@ -169,6 +170,12 @@ def close(self):
for archive in self._archives:
archive.close()

# Symbolicator/Python A/B testing
try:
self.perform_ab_test()
except Exception as e:
sentry_sdk.capture_exception(e)

def handles_frame(self, frame, stacktrace_info):
key = self._deep_freeze(frame)
self._proguard_processor_handles_frame[key] = self.proguard_processor.handles_frame(
Expand Down Expand Up @@ -282,6 +289,60 @@ def process_frame(self, processable_frame, processing_task):

return new_frames, raw_frames, processing_errors

def perform_ab_test(self):
symbolicator_stacktraces = self.data.pop("symbolicator_stacktraces", None)

if symbolicator_stacktraces is None:
return

# This should always be set if symbolicator_stacktraces is, but better safe than sorry
symbolicator_exceptions = self.data.pop("symbolicator_exceptions", ())

def frames_differ(a, b):
return (
a.get("lineno") != b.get("lineno")
or a.get("abs_path") != b.get("abs_path")
or a.get("function") != b.get("function")
or a.get("filename") != b.get("filename")
or a.get("module") != b.get("module")
or a.get("in_app") != b.get("in_app")
or a.get("context_line") != b.get("context_line")
)

def exceptions_differ(a, b):
return a.get("type") != b.get("type") or a.get("module") != b.get("module")

different_frames = []
for symbolicator_stacktrace, stacktrace_info in zip(
symbolicator_stacktraces, self.stacktrace_infos
):
# NOTE: lets hope that `stacktrace_info` has the already processed frames
python_stacktrace = stacktrace_info.container.get("stacktrace")

for symbolicator_frame, python_frame in zip(
symbolicator_stacktrace, python_stacktrace["frames"]
):
if frames_differ(symbolicator_frame, python_frame):
different_frames.append((symbolicator_frame, python_frame))

different_exceptions = []
for symbolicator_exception, python_exception in zip(
symbolicator_exceptions,
get_path(self.data, "exception", "values", filter=True, default=()),
):
if exceptions_differ(symbolicator_exception, python_exception):
different_exceptions.append((symbolicator_exception, python_exception))

if different_frames or different_exceptions:
with sentry_sdk.push_scope() as scope:
scope.set_extra("different_frames", different_frames)
scope.set_extra("different_exceptions", different_exceptions)
scope.set_extra("event_id", self.data.get("event_id"))
scope.set_tag("project_id", self.project.id)
sentry_sdk.capture_message(
"JVM symbolication differences between symbolicator and python."
)


class JavaPlugin(Plugin2):
can_disable = False
Expand Down
39 changes: 31 additions & 8 deletions src/sentry/lang/java/processing.py
Expand Up @@ -2,7 +2,11 @@
import re
from typing import Any

from sentry.lang.java.utils import get_jvm_images, get_proguard_images
from sentry.lang.java.utils import (
do_proguard_processing_ab_test,
get_jvm_images,
get_proguard_images,
)
from sentry.lang.native.error import SymbolicationFailed, write_error
from sentry.lang.native.symbolicator import Symbolicator
from sentry.models.eventerror import EventError
Expand Down Expand Up @@ -212,18 +216,20 @@ def process_jvm_stacktraces(symbolicator: Symbolicator, data: Any) -> Any:
if not _handle_response_status(data, response):
return

data["processed_by_symbolicator"] = True
should_do_ab_test = do_proguard_processing_ab_test()
symbolicator_exceptions: list[dict[str, Any]] = []
symbolicator_stacktraces: list[list[dict[str, Any]]] = []

processing_errors = response.get("errors", [])
if len(processing_errors) > 0:
if len(processing_errors) > 0 and not should_do_ab_test:
data.setdefault("errors", []).extend(map_symbolicator_process_jvm_errors(processing_errors))

assert len(stacktraces) == len(response["stacktraces"]), (stacktraces, response)

for sinfo, complete_stacktrace in zip(stacktrace_infos, response["stacktraces"]):
raw_frames = sinfo.stacktrace["frames"]
complete_frames = complete_stacktrace["frames"]
sinfo.stacktrace["frames"] = []
new_frames = []

for index, raw_frame in enumerate(raw_frames):
# If symbolicator returned any matching frames for this raw_frame, use them,
Expand All @@ -233,9 +239,14 @@ def process_jvm_stacktraces(symbolicator: Symbolicator, data: Any) -> Any:
for returned in matching_frames:
new_frame = dict(raw_frame)
_merge_frame(new_frame, returned)
sinfo.stacktrace["frames"].append(new_frame)
new_frames.append(new_frame)
else:
sinfo.stacktrace["frames"].append(raw_frame)
new_frames.append(raw_frame)

if should_do_ab_test:
symbolicator_stacktraces.append(new_frames)
else:
sinfo.stacktrace["frames"] = new_frames

if sinfo.container is not None:
sinfo.container["raw_stacktrace"] = {
Expand All @@ -249,7 +260,19 @@ def process_jvm_stacktraces(symbolicator: Symbolicator, data: Any) -> Any:
get_path(data, "exception", "values", filter=True, default=()),
response["exceptions"],
):
exception["type"] = complete_exception["type"]
exception["module"] = complete_exception["module"]
if should_do_ab_test:
new_exception = dict(exception)
new_exception["type"] = complete_exception["type"]
new_exception["module"] = complete_exception["module"]
symbolicator_exceptions.append(new_exception)
else:
exception["type"] = complete_exception["type"]
exception["module"] = complete_exception["module"]

if should_do_ab_test:
data["symbolicator_stacktraces"] = symbolicator_stacktraces
data["symbolicator_exceptions"] = symbolicator_exceptions
else:
data["processed_by_symbolicator"] = True

return data
7 changes: 6 additions & 1 deletion src/sentry/lang/java/utils.py
Expand Up @@ -7,7 +7,7 @@

from sentry import options
from sentry.attachments import CachedAttachment, attachment_cache
from sentry.features.rollout import in_rollout_group
from sentry.features.rollout import in_random_rollout, in_rollout_group
from sentry.ingest.consumer.processors import CACHE_TIMEOUT
from sentry.lang.java.proguard import open_proguard_mapper
from sentry.models.debugfile import ProjectDebugFile
Expand Down Expand Up @@ -141,6 +141,7 @@ def deobfuscate_view_hierarchy(data):

SYMBOLICATOR_PROGUARD_PROJECTS_OPTION = "symbolicator.proguard-processing-projects"
SYMBOLICATOR_PROGUARD_SAMPLE_RATE_OPTION = "symbolicator.proguard-processing-sample-rate"
SYMBOLICATOR_PROGUARD_AB_TEST_OPTION = "symbolicator.proguard-processing-ab-test"


def should_use_symbolicator_for_proguard(project_id: int) -> bool:
Expand All @@ -162,3 +163,7 @@ def is_jvm_event(data: Any, stacktraces: list[StacktraceInfo]) -> bool:
return True

return False


def do_proguard_processing_ab_test() -> bool:
return in_random_rollout(SYMBOLICATOR_PROGUARD_AB_TEST_OPTION)
1 change: 1 addition & 0 deletions src/sentry/options/defaults.py
Expand Up @@ -628,6 +628,7 @@
register("symbolicator.proguard-processing-projects", type=Sequence, default=[])
# Enable use of Symbolicator proguard processing for fraction of projects.
register("symbolicator.proguard-processing-sample-rate", default=0.0)
register("symbolicator.proguard-processing-ab-test", default=0.0)

# Post Process Error Hook Sampling
register(
Expand Down

0 comments on commit b34eaf0

Please sign in to comment.