ref(grouping): Added initial pass of new grouping algorithm

Makefile

@@ -149,6 +149,18 @@ lint-python:
 	bash -eo pipefail -c "flake8 | tee .artifacts/flake8.pycodestyle.log"
 	@echo ""
 
+review-python-snapshots:
+	@cargo insta --version &> /dev/null || cargo install cargo-insta
+	@cargo insta review --workspace-root `pwd` -e pysnap
+
+accept-python-snapshots:
+	@cargo insta --version &> /dev/null || cargo install cargo-insta
+	@cargo insta accept --workspace-root `pwd` -e pysnap
+
+reject-python-snapshots:
+	@cargo insta --version &> /dev/null || cargo install cargo-insta
+	@cargo insta reject --workspace-root `pwd` -e pysnap
+
 lint-js:
 	@echo "--> Linting javascript"
 	bin/lint --js --parseable

src/sentry/grouping/strategies/base.py

@@ -1,5 +1,7 @@
 from __future__ import absolute_import
 
+import inspect
+
 from sentry.grouping.component import GroupingComponent
 
 
@@ -158,10 +160,11 @@ def get_grouping_component_variants(self, event, config=None):
 
 class StrategyConfiguration(object):
 
-    def __init__(self, id, strategies, delegates=None):
+    def __init__(self, id, strategies, delegates=None, changelog=None):
         self.id = id
         self.strategies = {}
         self.delegates = {}
+        self.changelog = inspect.cleandoc(changelog or '')
 
         for strategy_id in strategies:
             strategy = lookup_strategy(strategy_id)

src/sentry/grouping/strategies/configurations.py

@@ -3,21 +3,61 @@
 from sentry.grouping.strategies.base import StrategyConfiguration
 
 
-# The latest version of th edefault config that should be used
+# The latest version of the default config that should be used
 DEFAULT_CONFIG = 'legacy:2019-03-12'
 
+# The classes of grouping algorithms
+CLASSES = []
+
 # The full mapping of all known configurations.
 CONFIGURATIONS = {}
 
 
-def register_strategy_config(id, strategies, delegates=None):
-    rv = StrategyConfiguration(id, strategies, delegates)
+def register_strategy_config(id, **kwargs):
+    rv = StrategyConfiguration(id, **kwargs)
+    cls = id.split(':', 1)[0]
+    if cls not in CLASSES:
+        CLASSES.append(cls)
     CONFIGURATIONS[rv.id] = rv
     return rv
 
 
+# Legacy groupings
+#
+# These we do not plan on changing much, but bugfixes here might still go
+# into new grouping versions.
+
 register_strategy_config(
     id='legacy:2019-03-12',
+    strategies=[
+        'expect-ct:v1',
+        'expect-staple:v1',
+        'hpkp:v1',
+        'csp:v1',
+        'threads:v1',
+        'stacktrace:legacy',
+        'chained-exception:legacy',
+        'template:v1',
+        'message:v1',
+    ],
+    delegates=[
+        'frame:legacy',
+        'stacktrace:legacy',
+        'single-exception:legacy',
+    ],
+    changelog='''
+        * Traditional grouping algorithm
+        * Some known weeknesses with regards to grouping of native frames
+    '''
+)
+
+# Newstyle grouping
+#
+# This is the new grouping strategy but it's not yet versioned because
+# it's not available to customers yet.
+
+register_strategy_config(
+    id='new:wip',
     strategies=[
         'expect-ct:v1',
         'expect-staple:v1',
@@ -33,5 +73,8 @@ def register_strategy_config(id, strategies, delegates=None):
         'frame:v1',
         'stacktrace:v1',
         'single-exception:v1',
-    ]
+    ],
+    changelog='''
+        * Work in progress grouping algorith that is not frozen in behavior yet
+    '''
 )

src/sentry/grouping/strategies/exception.py

@@ -1,7 +1,5 @@
 from __future__ import absolute_import
 
-import six
-
 from sentry.grouping.component import GroupingComponent
 from sentry.grouping.strategies.base import strategy
 
@@ -15,38 +13,19 @@ def single_exception_v1(exception, config, **meta):
     type_component = GroupingComponent(
         id='type',
         values=[exception.type] if exception.type else [],
-        contributes=False
-    )
-    value_component = GroupingComponent(
-        id='value',
-        values=[exception.value] if exception.value else [],
-        contributes=False
     )
-    stacktrace_component = GroupingComponent(id='stacktrace')
 
     if exception.stacktrace is not None:
         stacktrace_component = config.get_grouping_component(
             exception.stacktrace, **meta)
-        if stacktrace_component.contributes:
-            if exception.type:
-                type_component.update(contributes=True)
-                if exception.value:
-                    value_component.update(hint='stacktrace and type take precedence')
-            elif exception.value:
-                value_component.update(hint='stacktrace takes precedence')
-
-    if not stacktrace_component.contributes:
-        if exception.type:
-            type_component.update(contributes=True)
-        if exception.value:
-            value_component.update(contributes=True)
+    else:
+        stacktrace_component = GroupingComponent(id='stacktrace')
 
     return GroupingComponent(
         id='exception',
         values=[
             stacktrace_component,
             type_component,
-            value_component,
         ]
     )
 
@@ -59,70 +38,16 @@ def single_exception_v1(exception, config, **meta):
 )
 def chained_exception_v1(chained_exception, config, **meta):
     # Case 1: we have a single exception, use the single exception
-    # component directly
+    # component directly to avoid a level of nesting
     exceptions = chained_exception.exceptions()
     if len(exceptions) == 1:
         return config.get_grouping_component(exceptions[0], **meta)
 
-    # Case 2: try to build a new component out of the individual
-    # errors however with a trick.  In case any exeption has a
-    # stacktrace we want to ignore all other exceptions.
-    any_stacktraces = False
     values = []
     for exception in exceptions:
-        exception_component = config.get_grouping_component(exception, **meta)
-        stacktrace_component = exception_component.get_subcomponent('stacktrace')
-        if stacktrace_component is not None and \
-           stacktrace_component.contributes:
-            any_stacktraces = True
-        values.append(exception_component)
-
-    if any_stacktraces:
-        for value in values:
-            stacktrace_component = value.get_subcomponent('stacktrace')
-            if stacktrace_component is None or not stacktrace_component.contributes:
-                value.update(
-                    contributes=False,
-                    hint='exception has no stacktrace',
-                )
+        values.append(config.get_grouping_component(exception, **meta))
 
     return GroupingComponent(
         id='chained-exception',
         values=values,
     )
-
-
-@chained_exception_v1.variant_processor
-def chained_exception_v1_variant_processor(variants, config, **meta):
-    if len(variants) <= 1:
-        return variants
-    any_stacktrace_contributes = False
-    non_contributing_components = []
-    stacktrace_variants = set()
-
-    # In case any of the variants has a contributing stacktrace, we want
-    # to make all other variants non contributing.  Thr e
-    for (key, component) in six.iteritems(variants):
-        if any(s.contributes for s in component.iter_subcomponents(
-                id='stacktrace', recursive=True)):
-            any_stacktrace_contributes = True
-            stacktrace_variants.add(key)
-        else:
-            non_contributing_components.append(component)
-
-    if any_stacktrace_contributes:
-        if len(stacktrace_variants) == 1:
-            hint_suffix = 'but the %s variant does' % next(iter(stacktrace_variants))
-        else:
-            # this branch is basically dead because we only have two
-            # variants right now, but this is so this does not break in
-            # the future.
-            hint_suffix = 'others do'
-        for component in non_contributing_components:
-            component.update(
-                contributes=False,
-                hint='ignored because this variant does not contain a '
-                'stacktrace, but %s' % hint_suffix
-            )
-
-    return variants

src/sentry/grouping/strategies/legacy_exception.py

@@ -0,0 +1,128 @@
+from __future__ import absolute_import
+
+import six
+
+from sentry.grouping.component import GroupingComponent
+from sentry.grouping.strategies.base import strategy
+
+
+@strategy(
+    id='single-exception:legacy',
+    interfaces=['singleexception'],
+    variants=['!system', 'app'],
+)
+def single_exception_legacy(exception, config, **meta):
+    type_component = GroupingComponent(
+        id='type',
+        values=[exception.type] if exception.type else [],
+        contributes=False
+    )
+    value_component = GroupingComponent(
+        id='value',
+        values=[exception.value] if exception.value else [],
+        contributes=False
+    )
+    stacktrace_component = GroupingComponent(id='stacktrace')
+
+    if exception.stacktrace is not None:
+        stacktrace_component = config.get_grouping_component(
+            exception.stacktrace, **meta)
+        if stacktrace_component.contributes:
+            if exception.type:
+                type_component.update(contributes=True)
+                if exception.value:
+                    value_component.update(hint='stacktrace and type take precedence')
+            elif exception.value:
+                value_component.update(hint='stacktrace takes precedence')
+
+    if not stacktrace_component.contributes:
+        if exception.type:
+            type_component.update(contributes=True)
+        if exception.value:
+            value_component.update(contributes=True)
+
+    return GroupingComponent(
+        id='exception',
+        values=[
+            stacktrace_component,
+            type_component,
+            value_component,
+        ]
+    )
+
+
+@strategy(
+    id='chained-exception:legacy',
+    interfaces=['exception'],
+    variants=['!system', 'app'],
+    score=2000,
+)
+def chained_exception_legacy(chained_exception, config, **meta):
+    # Case 1: we have a single exception, use the single exception
+    # component directly
+    exceptions = chained_exception.exceptions()
+    if len(exceptions) == 1:
+        return config.get_grouping_component(exceptions[0], **meta)
+
+    # Case 2: try to build a new component out of the individual
+    # errors however with a trick.  In case any exeption has a
+    # stacktrace we want to ignore all other exceptions.
+    any_stacktraces = False
+    values = []
+    for exception in exceptions:
+        exception_component = config.get_grouping_component(exception, **meta)
+        stacktrace_component = exception_component.get_subcomponent('stacktrace')
+        if stacktrace_component is not None and \
+           stacktrace_component.contributes:
+            any_stacktraces = True
+        values.append(exception_component)
+
+    if any_stacktraces:
+        for value in values:
+            stacktrace_component = value.get_subcomponent('stacktrace')
+            if stacktrace_component is None or not stacktrace_component.contributes:
+                value.update(
+                    contributes=False,
+                    hint='exception has no stacktrace',
+                )
+
+    return GroupingComponent(
+        id='chained-exception',
+        values=values,
+    )
+
+
+@chained_exception_legacy.variant_processor
+def chained_exception_legacy_variant_processor(variants, config, **meta):
+    if len(variants) <= 1:
+        return variants
+    any_stacktrace_contributes = False
+    non_contributing_components = []
+    stacktrace_variants = set()
+
+    # In case any of the variants has a contributing stacktrace, we want
+    # to make all other variants non contributing.  Thr e
+    for (key, component) in six.iteritems(variants):
+        if any(s.contributes for s in component.iter_subcomponents(
+                id='stacktrace', recursive=True)):
+            any_stacktrace_contributes = True
+            stacktrace_variants.add(key)
+        else:
+            non_contributing_components.append(component)
+
+    if any_stacktrace_contributes:
+        if len(stacktrace_variants) == 1:
+            hint_suffix = 'but the %s variant does' % next(iter(stacktrace_variants))
+        else:
+            # this branch is basically dead because we only have two
+            # variants right now, but this is so this does not break in
+            # the future.
+            hint_suffix = 'others do'
+        for component in non_contributing_components:
+            component.update(
+                contributes=False,
+                hint='ignored because this variant does not contain a '
+                'stacktrace, but %s' % hint_suffix
+            )
+
+    return variants