ref(grouping): Use underscores for variant types and keys (#80497)

In order to make usage in an upcoming `TypedDict` easier, this switches the strings we use as keys for our grouping variants from using dashes to using underscores. (It also changes said variants' `type` attributes, since they are meant to match the keys.) Because this is a somewhat noisy change, given the snapshot updates it causes, it's been split out into its own PR. Note: These keys are never stored, so making the switch doesn't cause a data consistency problem - mostly we just pass them around the BE while we calculate event grouping. That said, we do pass them as part of the response from the grouping info endpoint. In order to preserve compatibility with the FE, this therefore switches the data back to using dashes just before it is returned from the endpoint. The fact that the `test_variant_keys_and_types_use_dashes_not_underscores` test[1] continues to pass with no changes proves that this conversion works. [1] https://github.com/getsentry/sentry/blob/6abc34dcff03a54e68946843642e07c594d5ee7c/tests/sentry/api/endpoints/test_event_grouping_info.py#L151-L178
getsentry · Nov 11, 2024 · fb831cd · fb831cd
1 parent f544264
commit fb831cd
Show file tree

Hide file tree

Showing 87 changed files with 222 additions and 207 deletions.
diff --git a/src/sentry/api/endpoints/event_grouping_info.py b/src/sentry/api/endpoints/event_grouping_info.py
@@ -29,7 +29,22 @@ def get(self, request: HttpRequest, project, event_id) -> HttpResponse:
         if event is None:
             raise ResourceDoesNotExist
 
-        grouping_info = get_grouping_info(request.GET.get("config", None), project, event)
+        _grouping_info = get_grouping_info(request.GET.get("config", None), project, event)
+
+        # TODO: All of the below is a temporary hack to preserve compatibility between the BE and FE as
+        # we transition from using dashes in the keys/variant types to using underscores. For now, until
+        # we change the FE, we switch back to dashes before sending the data.
+        grouping_info = {}
+
+        for key, variant_dict in _grouping_info.items():
+            new_key = key.replace("_", "-")
+            new_type = variant_dict.get("type", "").replace("_", "-")
+
+            variant_dict["key"] = new_key
+            if "type" in variant_dict:
+                variant_dict["type"] = new_type
+
+            grouping_info[new_key] = variant_dict
 
         return HttpResponse(
             orjson.dumps(grouping_info, option=orjson.OPT_UTC_Z), content_type="application/json"

diff --git a/src/sentry/grouping/api.py b/src/sentry/grouping/api.py
@@ -321,7 +321,7 @@ def get_grouping_variants_for_event(
             return {"checksum": ChecksumVariant(checksum)}
 
         rv: dict[str, BaseVariant] = {
-            "hashed-checksum": HashedChecksumVariant(hash_from_values(checksum), checksum),
+            "hashed_checksum": HashedChecksumVariant(hash_from_values(checksum), checksum),
         }
 
         # The legacy code path also supported arbitrary values here but
@@ -360,9 +360,9 @@ def get_grouping_variants_for_event(
 
         fingerprint = resolve_fingerprint_values(fingerprint, event.data)
         if (fingerprint_info or {}).get("matched_rule", {}).get("is_builtin") is True:
-            rv["built-in-fingerprint"] = BuiltInFingerprintVariant(fingerprint, fingerprint_info)
+            rv["built_in_fingerprint"] = BuiltInFingerprintVariant(fingerprint, fingerprint_info)
         else:
-            rv["custom-fingerprint"] = CustomFingerprintVariant(fingerprint, fingerprint_info)
+            rv["custom_fingerprint"] = CustomFingerprintVariant(fingerprint, fingerprint_info)
 
     # If only the default is referenced, we can use the variants as is
     elif defaults_referenced == 1 and len(fingerprint) == 1:

diff --git a/src/sentry/grouping/ingest/grouphash_metadata.py b/src/sentry/grouping/ingest/grouphash_metadata.py
@@ -78,10 +78,10 @@ def _get_hash_basis(event: Event, project: Project, variants: dict[str, BaseVari
         # variants
         if "app" in variants and variants["app"].contributes
         else (
-            variants["hashed-checksum"]
+            variants["hashed_checksum"]
             # TODO: We won't need this 'if' once we stop returning both hashed and non-hashed
             # checksum contributing variants
-            if "hashed-checksum" in variants
+            if "hashed_checksum" in variants
             # Other than in the broken app/system and hashed/raw checksum cases, there should only
             # ever be a single contributing variant
             else [variant for variant in variants.values() if variant.contributes][0]

diff --git a/src/sentry/grouping/ingest/seer.py b/src/sentry/grouping/ingest/seer.py
@@ -98,7 +98,7 @@ def _has_customized_fingerprint(event: Event, variants: dict[str, BaseVariant])
             return True
 
     # Fully customized fingerprint (from either us or the user)
-    fingerprint_variant = variants.get("custom-fingerprint") or variants.get("built-in-fingerprint")
+    fingerprint_variant = variants.get("custom_fingerprint") or variants.get("built_in_fingerprint")
 
     if fingerprint_variant:
         metrics.incr(

diff --git a/src/sentry/grouping/variants.py b/src/sentry/grouping/variants.py
@@ -55,7 +55,7 @@ def _get_metadata_as_dict(self):
 
 
 class HashedChecksumVariant(ChecksumVariant):
-    type = "hashed-checksum"
+    type = "hashed_checksum"
     description = "hashed legacy checksum"
 
     def __init__(self, checksum: str, raw_checksum: str):
@@ -85,7 +85,7 @@ class PerformanceProblemVariant(BaseVariant):
         contains the event and the evidence.
     """
 
-    type = "performance-problem"
+    type = "performance_problem"
     description = "performance problem"
     contributes = True
 
@@ -157,7 +157,7 @@ def expose_fingerprint_dict(values, info=None):
 class CustomFingerprintVariant(BaseVariant):
     """A user-defined custom fingerprint."""
 
-    type = "custom-fingerprint"
+    type = "custom_fingerprint"
 
     def __init__(self, values, fingerprint_info=None):
         self.values = values
@@ -177,7 +177,7 @@ def _get_metadata_as_dict(self):
 class BuiltInFingerprintVariant(CustomFingerprintVariant):
     """A built-in, Sentry defined fingerprint."""
 
-    type = "built-in-fingerprint"
+    type = "built_in_fingerprint"
 
     @property
     def description(self):
@@ -187,7 +187,7 @@ def description(self):
 class SaltedComponentVariant(ComponentVariant):
     """A salted version of a component."""
 
-    type = "salted-component"
+    type = "salted_component"
 
     def __init__(self, values, component, config, fingerprint_info=None):
         ComponentVariant.__init__(self, component, config)

diff --git a/...s/test_fingerprinting/test_event_hash_variant/built_in_fingerprint_chunkload_error.pysnap b/...s/test_fingerprinting/test_event_hash_variant/built_in_fingerprint_chunkload_error.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-15T17:19:43.335885+00:00'
+created: '2024-11-08T22:03:15.369457+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -15,9 +15,9 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  built-in-fingerprint:
+  built_in_fingerprint:
     matched_rule: family:"javascript" type:"ChunkLoadError" -> "chunkloaderror"
-    type: built-in-fingerprint
+    type: built_in_fingerprint
     values:
     - chunkloaderror
   system:

diff --git a/...rouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_abs_path.pysnap b/...rouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_abs_path.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:30.800129+00:00'
+created: '2024-11-08T22:03:15.648004+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -22,9 +22,9 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  custom-fingerprint:
+  custom_fingerprint:
     matched_rule: type:"DatabaseUnavailable" -> "{{ stack.abs_path }}"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - /foo/Application.cpp
   system:

diff --git a/...ing/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_escape_chars.pysnap b/...ing/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_escape_chars.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:31.788940+00:00'
+created: '2024-11-08T22:03:16.573246+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -19,9 +19,9 @@ fingerprint:
 - '{{ message }}'
 title: '{[*?]}'
 variants:
-  custom-fingerprint:
+  custom_fingerprint:
     matched_rule: message:"\{\[\*\?\]\}" -> "escaped{{ message }}"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - escaped
     - '{[*?]}'

diff --git a/...g/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_exception_type.pysnap b/...g/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_exception_type.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:29.876535+00:00'
+created: '2024-11-08T22:03:15.088186+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -22,12 +22,12 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  custom-fingerprint:
+  custom_fingerprint:
     client_values:
     - my-route
     - '{{ default }}'
     matched_rule: type:"DatabaseUnavailable" -> "database-unavailable"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - database-unavailable
   system:

diff --git a/.../test_fingerprinting/test_event_hash_variant/fingerprint_exception_type_and_module.pysnap b/.../test_fingerprinting/test_event_hash_variant/fingerprint_exception_type_and_module.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:32.297082+00:00'
+created: '2024-11-08T22:03:17.224512+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -24,12 +24,12 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  custom-fingerprint:
+  custom_fingerprint:
     client_values:
     - my-route
     - '{{ default }}'
     matched_rule: type:"DatabaseUnavailable" module:"io.sentry.example.*" -> "database-unavailable"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - database-unavailable
   system:

diff --git a/...test_fingerprinting/test_event_hash_variant/fingerprint_exception_type_and_module2.pysnap b/...test_fingerprinting/test_event_hash_variant/fingerprint_exception_type_and_module2.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:28.545402+00:00'
+created: '2024-11-08T22:03:13.965272+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -27,7 +27,7 @@ variants:
     component:
       contributes: false
       hint: exception of system takes precedence
-    type: salted-component
+    type: salted_component
     values:
     - my-route
     - '{{ default }}'
@@ -38,7 +38,7 @@ variants:
     component:
       contributes: true
       hint: null
-    type: salted-component
+    type: salted_component
     values:
     - my-route
     - '{{ default }}'
diff --git a/...ots/test_fingerprinting/test_event_hash_variant/fingerprint_exception_type_and_sdk.pysnap b/...ots/test_fingerprinting/test_event_hash_variant/fingerprint_exception_type_and_sdk.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:28.751675+00:00'
+created: '2024-11-08T22:03:14.259249+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -24,12 +24,12 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  custom-fingerprint:
+  custom_fingerprint:
     client_values:
     - my-route
     - '{{ default }}'
     matched_rule: sdk:"sentry.java" type:"DatabaseUnavailable" -> "database-unavailable"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - database-unavailable
   system:

diff --git a/...fingerprinting/test_event_hash_variant/fingerprint_exception_type_and_sdk_mismatch.pysnap b/...fingerprinting/test_event_hash_variant/fingerprint_exception_type_and_sdk_mismatch.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:31.555178+00:00'
+created: '2024-11-08T22:03:16.302941+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -27,7 +27,7 @@ variants:
     component:
       contributes: false
       hint: exception of system takes precedence
-    type: salted-component
+    type: salted_component
     values:
     - my-route
     - '{{ default }}'
@@ -38,7 +38,7 @@ variants:
     component:
       contributes: true
       hint: null
-    type: salted-component
+    type: salted_component
     values:
     - my-route
     - '{{ default }}'
diff --git a/.../snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_exception_value.pysnap b/.../snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_exception_value.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:32.087925+00:00'
+created: '2024-11-08T22:03:16.952667+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -24,9 +24,9 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  custom-fingerprint:
+  custom_fingerprint:
     matched_rule: value:"*went wrong*" -> "something-went-wrong{{ error.value }}"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - something-went-wrong
     - something went WRONG

diff --git a/...rouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_function.pysnap b/...rouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_function.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:28.227754+00:00'
+created: '2024-11-08T22:03:13.661245+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -27,10 +27,10 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  custom-fingerprint:
+  custom_fingerprint:
     matched_rule: type:"DatabaseUnavailable" module:"io.sentry.example.*" -> "database-unavailable{{
       function }}"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - database-unavailable
     - main

diff --git a/...grouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_message.pysnap b/...grouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_message.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:31.887130+00:00'
+created: '2024-11-08T22:03:16.698987+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -19,9 +19,9 @@ fingerprint:
 - '{{ message }}'
 title: Hello my sweet Love
 variants:
-  custom-fingerprint:
+  custom_fingerprint:
     matched_rule: message:"*love*" -> "what-is-love{{ message }}"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - what-is-love
     - Hello my sweet Love

diff --git a/...snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_message_on_value.pysnap b/...snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_message_on_value.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:28.849683+00:00'
+created: '2024-11-08T22:03:14.418050+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -24,9 +24,9 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  custom-fingerprint:
+  custom_fingerprint:
     matched_rule: message:"*love*" -> "what-is-love{{ message }}"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - what-is-love
     - something has no love.

diff --git a/.../grouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_native.pysnap b/.../grouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_native.pysnap
@@ -1,5 +1,5 @@
 ---
-created: '2024-10-16T22:32:30.109890+00:00'
+created: '2024-11-08T22:03:15.258775+00:00'
 creator: sentry
 source: tests/sentry/grouping/test_fingerprinting.py
 ---
@@ -24,10 +24,10 @@ variants:
       contributes: false
       hint: custom fingerprint takes precedence
     type: component
-  custom-fingerprint:
+  custom_fingerprint:
     matched_rule: type:"SymCacheError" function:"symbolicator::actors::symcaches::*"
       -> "symcache-error"
-    type: custom-fingerprint
+    type: custom_fingerprint
     values:
     - symcache-error
   system: