Skip to content

Commit

Permalink
bug 1762207: remove hang_type
Browse files Browse the repository at this point in the history
hang_type is based on Hang and HangID crash annotations which were
removed 10 years ago. This removes them from Socorro.
  • Loading branch information
willkg committed Apr 4, 2022
1 parent ce503f5 commit 8fffe75
Show file tree
Hide file tree
Showing 19 changed files with 13 additions and 203 deletions.
20 changes: 0 additions & 20 deletions socorro/external/es/super_search_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -1950,26 +1950,6 @@ def number_field(
"query_type": "bool",
"storage_mapping": {"type": "boolean"},
},
"hang_type": {
"data_validation_type": "enum",
"description": (
"Tells if a report was caused by a crash or a hang. In the database, the value "
"is `0` if the problem was a crash of the software, and `1` or `-1` if the problem "
"was a hang of the software. \n\nNote: for querying, you should use `crash` or "
"`hang`, since those are automatically transformed into the correct underlying "
"values."
),
"form_field_choices": ["any", "crash", "hang", "all"],
"has_full_version": False,
"in_database_name": "hang_type",
"is_exposed": True,
"is_returned": True,
"name": "hang_type",
"namespace": "processed_crash",
"permissions_needed": [],
"query_type": "enum",
"storage_mapping": {"type": "short"},
},
"has_device_touch_screen": {
"data_validation_type": "bool",
"description": (
Expand Down
24 changes: 0 additions & 24 deletions socorro/lib/search_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ def get_parameters(self, **kwargs):
parameters[value.name].append(value)

self.fix_date_parameter(parameters)
self.fix_hang_type_parameter(parameters)
self.fix_version_parameter(parameters)

return parameters
Expand Down Expand Up @@ -289,29 +288,6 @@ def fix_date_parameter(self, parameters):
parameters["date"].append(lower_than)
parameters["date"].append(greater_than)

@staticmethod
def fix_hang_type_parameter(parameters):
"""Correct the hang_type parameter.
If hang_type is 'crash', replace it with '0', if it's 'hang' replace it
with '-1, 1'.
"""
if not parameters.get("hang_type"):
return

for hang_type in parameters["hang_type"]:
new_values = []
for val in hang_type.value:
if val == "crash":
new_values.append(0)
elif val == "hang":
new_values.extend([-1, 1])
else:
new_values.append(val)

hang_type.value = new_values
hang_type.data_type = "int"

@staticmethod
def fix_version_parameter(parameters):
"""Correct the version parameter.
Expand Down
22 changes: 1 addition & 21 deletions socorro/processor/rules/mozilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,29 +255,9 @@ def action(self, raw_crash, dumps, processed_crash, processor_meta):


class PluginRule(Rule):
"""Handle plugin-related things and hang_type."""
"""Handle plugin-related fields."""

def action(self, raw_crash, dumps, processed_crash, processor_meta):
processed_crash["hangid"] = raw_crash.get("HangID", None)

# the processed_crash["hang_type"] has the following meaning:
#
# * hang_type == -1 is a plugin hang
# * hang_type == 1 is a parent hang
# * hang_type == 0 is not a hang at all, but a normal crash

try:
hang_as_int = int(raw_crash.get("Hang", False))
except ValueError:
hang_as_int = 0

if hang_as_int:
processed_crash["hang_type"] = 1
elif processed_crash["hangid"]:
processed_crash["hang_type"] = -1
else:
processed_crash["hang_type"] = 0

process_type = raw_crash.get("ProcessType", "parent")
if process_type == "plugin":
processed_crash["plugin_filename"] = raw_crash.get("PluginFilename", "")
Expand Down
2 changes: 1 addition & 1 deletion socorro/schemas/telemetry_socorro_crash.json
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@
"integer",
"null"
],
"description": "Tells if a report was caused by a crash or a hang. In the database, the value is `0` if the problem was a crash of the software, and `1` or `-1` if the problem was a hang of the software. \n\nNote: for querying, you should use `crash` or `hang`, since those are automatically transformed into the correct underlying values."
"description": "DEPRECATED EMPTY FIELD. See bug #1762207."
},
"install_age": {
"type": [
Expand Down
12 changes: 1 addition & 11 deletions socorro/signature/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ def normalize_frame(
# Return module/module_offset
return "{}@{}".format(module or "", strip_leading_zeros(module_offset))

def generate(self, source_list, hang_type=0, crashed_thread=None, delimiter=" | "):
def generate(self, source_list, crashed_thread=None, delimiter=" | "):
"""Iterate over frames in the crash stack and generate a signature.
First, we look for a sentinel frame and if we find one, we start with that.
Expand Down Expand Up @@ -296,15 +296,6 @@ def generate(self, source_list, hang_type=0, crashed_thread=None, delimiter=" |

debug_notes.append(f'prefix; continue iterating: "{a_signature}"')

# Add a special marker for hang crash reports.
if hang_type:
debug_notes.append(
"hang_type {}: prepending {}".format(
hang_type, self.hang_prefixes[hang_type]
)
)
new_signature_list.insert(0, self.hang_prefixes[hang_type])

signature = delimiter.join(new_signature_list)

# Handle empty signatures to explain why we failed generating them.
Expand Down Expand Up @@ -545,7 +536,6 @@ def action(self, crash_data, result):

signature, notes, debug_notes = self.c_signature_tool.generate(
signature_list,
crash_data.get("hang_type"),
crash_data.get("crashing_thread"),
)

Expand Down
28 changes: 0 additions & 28 deletions socorro/signature/tests/test_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,25 +229,6 @@ def test_generate_1(self):
sig, notes, debug_notes = s.generate(a)
assert sig == "d | e | f | g"

def test_generate_2(self):
"""test_generate_2: hang"""
s = self.setup_config_c_sig_tool(ig=["a", "b", "c"], pr=["d", "e", "f"])
a = list("abcdefghijklmnopqrstuvwxyz")
sig, notes, debug_notes = s.generate(a, hang_type=-1)
assert sig == "hang | d | e | f | g"

a = list("abcdaeafagahijklmnopqrstuvwxyz")
sig, notes, debug_notes = s.generate(a, hang_type=-1)
assert sig == "hang | d | e | f | g"

a = list("abcdaeafagahijklmnopqrstuvwxyz")
sig, notes, debug_notes = s.generate(a, hang_type=0)
assert sig == "d | e | f | g"

a = list("abcdaeafagahijklmnopqrstuvwxyz")
sig, notes, debug_notes = s.generate(a, hang_type=1)
assert sig == "chromehang | d | e | f | g"

def test_generate_2a(self):
"""test_generate_2a: way too long"""
s = self.setup_config_c_sig_tool(ig=["a", "b", "c"], pr=["d", "e", "f"])
Expand All @@ -265,15 +246,6 @@ def test_generate_2a(self):
)
sig, notes, debug_notes = s.generate(a)
assert sig == expected
expected = (
"hang | "
"dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd | "
"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee | "
"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff | "
"gggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg"
)
sig, notes, debug_notes = s.generate(a, hang_type=-1)
assert sig == expected

def test_generate_3(self):
"""test_generate_3: simple sentinel"""
Expand Down
4 changes: 0 additions & 4 deletions socorro/signature/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ def test_int_or_none(data, expected):
({}, 0),
({"crashing_thread": ""}, 0),
({"crashing_thread": "5"}, 5),
({"hang_type": 0, "crashing_thread": "5"}, 5),
# If hang_type == 1, the crashing_thread is always 0
({"hang_type": 1}, 0),
({"hang_type": 1, "crashing_thread": "5"}, 0),
],
)
def test_get_crashing_thread(crash_data, expected):
Expand Down
17 changes: 4 additions & 13 deletions socorro/signature/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,15 @@ def get_crashing_thread(crash_data):
"""
Takes crash data and returns the crashing thread as an int.
If it's a chrome hang (hang_type=1), then use thread 0. Otherwise, use the crashing
thread specified in the crash data.
:arg crash_data: crash data structure that conforms to the schema
:returns: crashing thread as an int
"""
crashing_thread = 0
if crash_data.get("hang_type", None) != 1:
try:
crashing_thread = int(crash_data.get("crashing_thread", 0))
except (TypeError, ValueError):
pass

return crashing_thread
try:
return int(crash_data.get("crashing_thread", 0))
except (TypeError, ValueError):
return 0


DOES_NOT_EXIST = object()
Expand Down Expand Up @@ -85,8 +78,6 @@ def convert_to_crash_data(processed_crash):
"reason": glom(processed_crash, "json_dump.crash_info.type", default=None),
# list of CStackTrace or None
"threads": glom(processed_crash, "json_dump.threads", default=None),
# int or None
"hang_type": glom(processed_crash, "hang_type", default=None),
# text or None
"os": glom(processed_crash, "json_dump.system_info.os", default=None),
# int or None
Expand Down
26 changes: 0 additions & 26 deletions socorro/unittest/lib/test_search_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,6 @@
"is_exposed": True,
"is_returned": True,
},
"hang_type": {
"name": "hang_type",
"data_validation_type": "enum",
"query_type": "enum",
"namespace": "processed_crash",
"permissions_needed": [],
"is_exposed": True,
"is_returned": True,
},
"user_comments": {
"name": "user_comments",
"data_validation_type": "str",
Expand Down Expand Up @@ -113,15 +104,13 @@ def test_get_parameters(self):
args = {
"signature": "~js",
"product": ["WaterWolf", "NightTrain"],
"hang_type": "=hang",
}
params = search.get_parameters(**args)
assert params["signature"][0].operator == "~"
assert params["signature"][0].value == "js"
assert params["product"][0].operator == ""
# Test that params with no operator are stacked
assert params["product"][0].value == ["WaterWolf", "NightTrain"]
assert params["hang_type"][0].operator == ""

args = {"signature": ["~Mark", "$js"]}
params = search.get_parameters(**args)
Expand Down Expand Up @@ -220,21 +209,6 @@ def test_get_parameters_date_max_range(self):
with pytest.raises(BadArgumentError):
search.get_parameters(date=">1999-01-01")

def test_hang_type_parameter_correction(self):
search = SearchBaseWithFields()

args = {"hang_type": "hang"}
params = search.get_parameters(**args)
assert "hang_type" in params
assert len(params["hang_type"]) == 1
assert params["hang_type"][0].value == [-1, 1]

args = {"hang_type": "crash"}
params = search.get_parameters(**args)
assert "hang_type" in params
assert len(params["hang_type"]) == 1
assert params["hang_type"][0].value == [0]

def test_version_parameter_correction(self):
search = SearchBaseWithFields()

Expand Down
5 changes: 0 additions & 5 deletions socorro/unittest/processor/rules/test_mozilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,6 @@ def test_no_process_type_is_parent(self):
class TestPluginRule:
def test_browser_hang(self):
raw_crash = {
"Hang": "1",
"ProcessType": "parent",
}
dumps = {}
Expand All @@ -436,8 +435,6 @@ def test_browser_hang(self):
rule = PluginRule()
rule.act(raw_crash, dumps, processed_crash, processor_meta)

assert processed_crash["hangid"] is None
assert processed_crash["hang_type"] == 1
assert "plugin_filename" not in processed_crash
assert "plugin_name" not in processed_crash
assert "plugin_version" not in processed_crash
Expand All @@ -457,8 +454,6 @@ def test_plugin_bits(self):
rule.act(raw_crash, dumps, processed_crash, processor_meta)

expected = {
"hang_type": 0,
"hangid": None,
"plugin_name": "name1",
"plugin_filename": "filename1",
"plugin_version": "1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,6 @@
/>
{%- endmacro %}

{% macro hang_crash_icon() %}
<img src="{{ static('img/3rdparty/fatcow/stop16x16.png') }}"
alt="stop sign"
title="Hanged Crash"
class="hang"
height="16"
width="16"
/>
{%- endmacro %}

{% macro plugin_crash_icon() %}
<img src="{{ static('img/3rdparty/fatcow/brick16x16.png') }}"
alt="brick"
Expand Down
2 changes: 0 additions & 2 deletions webapp-django/crashstats/crashstats/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -649,8 +649,6 @@ class ProcessedCrash(SocorroMiddleware):
"date_processed",
"dump",
"flash_version",
"hangid",
"hang_type",
"id",
"install_age",
"java_exception",
Expand Down
2 changes: 0 additions & 2 deletions webapp-django/crashstats/crashstats/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ def test_SignatureStats(self):
"is_garbage_collecting": [],
"process_type": [],
"platform": [{"count": 2, "term": ""}],
"hang_type": [{"count": 2, "term": 0}],
},
}
platforms = [
Expand Down Expand Up @@ -446,7 +445,6 @@ def test_SignatureStats(self):
assert signature_stats.is_startup_crash == 0
assert signature_stats.is_potential_startup_crash == 0
assert signature_stats.is_startup_window_crash is True
assert signature_stats.is_hang_crash is False
assert signature_stats.is_plugin_crash is False


Expand Down
10 changes: 0 additions & 10 deletions webapp-django/crashstats/crashstats/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,16 +197,6 @@ def is_startup_window_crash(self):
is_startup_window_crash = ratio > 0.5
return is_startup_window_crash

@cached_property
def is_hang_crash(self):
num_hang_crashes = 0
for row in self.signature["facets"]["hang_type"]:
# Hangs have weird values in the database: a value of 1 or -1
# means it is a hang, a value of 0 or missing means it is not.
if row["term"] in (1, -1):
num_hang_crashes += row["count"]
return num_hang_crashes > 0

@cached_property
def is_plugin_crash(self):
for row in self.signature["facets"]["process_type"]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
potential_fission_icon,
potential_startup_crash_icon,
potential_startup_window_crash_icon,
hang_crash_icon, plugin_crash_icon,
plugin_crash_icon,
browser_crash_icon %}

{% if signature_stats %}
Expand Down Expand Up @@ -41,13 +41,6 @@
</div>
{% endif %}

{% if signature_stats.is_hang_crash %}
<div class="crash-type-indicator-container">
{{ hang_crash_icon() }}
Hanged Crash
</div>
{% endif %}

{% if signature_stats.is_plugin_crash %}
<div class="crash-type-indicator-container">
{{ plugin_crash_icon() }}
Expand Down
1 change: 0 additions & 1 deletion webapp-django/crashstats/signature/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,6 @@ def signature_summary(request, params):

params["signature"] = "=" + params["signature"][0]
params["_aggs.signature"] = [
"hang_type",
"process_type",
"startup_crash",
"dom_fission_enabled",
Expand Down
Loading

0 comments on commit 8fffe75

Please sign in to comment.