Skip to content

Commit ebddea7

Browse files
authored
fix(grouping): Only collect metadata timing metric when actually getting metadata (#81252)
In #81070, a metric was added to time the gathering of grouphash metadata. Unfortunately, it's timing also all the cases in which we don't have to gather such data as well (which are the vast, vast majority of cases - for example, in the last 24 hours, we’ve hit roughly 2.5 million cases where the function it's timing has no-opped and only about 400 in which the function's actually had to do any work). As a result, the actual timing data we care about is being utterly drowned out. This fixes that by moving the timer from wrapping `create_or_update_grouphash_metadata` to wrapping it's helper, `get_hash_basis_and_metadata`, which only runs when the calculation is needed. It also renames the function we have been timing from `create_or_update_grouphash_metadata` to `create_or_update_grouphash_metadata_if_needed`, to make it clearer that it's just a wrapper which may no-op. (It's this unclear naming on my part which led to the confusion in the first place.)
1 parent 7e3f4c3 commit ebddea7

File tree

3 files changed

+19
-9
lines changed

3 files changed

+19
-9
lines changed

src/sentry/grouping/ingest/grouphash_metadata.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
TemplateHashingMetadata,
4242
)
4343
from sentry.utils import metrics
44+
from sentry.utils.metrics import MutableTags
4445

4546
logger = logging.getLogger(__name__)
4647

@@ -93,7 +94,7 @@
9394
}
9495

9596

96-
def create_or_update_grouphash_metadata(
97+
def create_or_update_grouphash_metadata_if_needed(
9798
event: Event,
9899
project: Project,
99100
grouphash: GroupHash,
@@ -105,7 +106,12 @@ def create_or_update_grouphash_metadata(
105106
# we'll have to override the metadata creation date for them.
106107

107108
if created:
108-
hash_basis, hashing_metadata = get_hash_basis_and_metadata(event, project, variants)
109+
with metrics.timer(
110+
"grouping.grouphashmetadata.get_hash_basis_and_metadata"
111+
) as metrics_timer_tags:
112+
hash_basis, hashing_metadata = get_hash_basis_and_metadata(
113+
event, project, variants, metrics_timer_tags
114+
)
109115

110116
GroupHashMetadata.objects.create(
111117
grouphash=grouphash,
@@ -121,7 +127,10 @@ def create_or_update_grouphash_metadata(
121127

122128

123129
def get_hash_basis_and_metadata(
124-
event: Event, project: Project, variants: dict[str, BaseVariant]
130+
event: Event,
131+
project: Project,
132+
variants: dict[str, BaseVariant],
133+
metrics_timer_tags: MutableTags,
125134
) -> tuple[HashBasis, HashingMetadata]:
126135
hashing_metadata: HashingMetadata = {}
127136

@@ -168,6 +177,8 @@ def get_hash_basis_and_metadata(
168177
)
169178
return (HashBasis.UNKNOWN, {})
170179

180+
metrics_timer_tags["hash_basis"] = hash_basis
181+
171182
# Gather different metadata depending on the grouping method
172183

173184
if hash_basis == HashBasis.STACKTRACE:

src/sentry/grouping/ingest/hashing.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
)
2323
from sentry.grouping.ingest.config import is_in_transition
2424
from sentry.grouping.ingest.grouphash_metadata import (
25-
create_or_update_grouphash_metadata,
25+
create_or_update_grouphash_metadata_if_needed,
2626
record_grouphash_metadata_metrics,
2727
)
2828
from sentry.grouping.variants import BaseVariant
@@ -233,10 +233,9 @@ def get_or_create_grouphashes(
233233
if options.get("grouping.grouphash_metadata.ingestion_writes_enabled") and features.has(
234234
"organizations:grouphash-metadata-creation", project.organization
235235
):
236-
with metrics.timer("grouping.grouphashmetadata.create_or_update_grouphash_metadata"):
237-
create_or_update_grouphash_metadata(
238-
event, project, grouphash, created, grouping_config, variants
239-
)
236+
create_or_update_grouphash_metadata_if_needed(
237+
event, project, grouphash, created, grouping_config, variants
238+
)
240239

241240
if grouphash.metadata:
242241
record_grouphash_metadata_metrics(grouphash.metadata)

tests/sentry/grouping/test_grouphash_metadata.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def _assert_and_snapshot_results(
135135
lines: list[str] = []
136136
variants = event.get_grouping_variants()
137137

138-
hash_basis, hashing_metadata = get_hash_basis_and_metadata(event, project, variants)
138+
hash_basis, hashing_metadata = get_hash_basis_and_metadata(event, project, variants, {})
139139

140140
with patch("sentry.grouping.ingest.grouphash_metadata.metrics.incr") as mock_metrics_incr:
141141
record_grouphash_metadata_metrics(

0 commit comments

Comments
 (0)