Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR getsentry#103425

Type: Corrupted (contains bugs)

Original PR Title: fix(grouping): Ensure custom titles use the correct frame
Original PR Description: This fixes a bug caused by getsentry#103268, which moved server-side fingerprinting before the call to normalize_stacktraces_for_grouping. While it's 99% true that they're unrelated processes (and therefore can happen in any order), the one exception to that is the handling of custom fingerprints which also include title information. In cases where the custom title includes any of the frame variables (function, module, package, or abs_path), the frame that gets used is the top in-app frame in the stacktrace. But in-app rules are applied as part of normalize_stacktraces_for_grouping, so having it not run until after the custom title is set is obviously a problem.

To fix this, the handling of such titles (in other words, the filling-in of the variables and adding of the result to the event) has been moved to live alongside the filling-in-of-the-variables which we do for the fingerprint itself, which is after in-app rules have been applied. A snapshot test illustrating the fix has also been added, showing that it's not the top frame but the top in-app frame which is used for the title.

(Why didn't I just switch the order of the two operations back, you ask? Because switching the order was the first step towards absorbing the normalization into variant calculation call which comes immediately after it, so server-side fingerprinting needed to get out of its original spot between them.)
Original PR URL: getsentry#103425


PR Type

Bug fix


Description

  • Move custom title application after stacktrace normalization

  • Ensure custom titles use top in-app frame, not top frame

  • Add helper function to apply custom titles from fingerprints

  • Add snapshot test validating correct frame selection


Diagram Walkthrough

flowchart LR
  A["Normalize stacktraces"] --> B["Apply server-side fingerprinting"]
  B --> C["Apply custom title from fingerprint"]
  C --> D["Event with correct title"]
Loading

File Walkthrough

Relevant files
Bug fix
api.py
Move custom title application to correct processing order

src/sentry/grouping/api.py

  • Removed custom title application from apply_server_side_fingerprinting
    function
  • Added new helper function _apply_custom_title_if_needed to apply
    custom titles after stacktrace normalization
  • Moved custom title logic to get_grouping_variants_for_event where it
    runs after in-app rules are applied
  • Added import for get_path utility function
+17/-8   
Tests
__init__.py
Update test helpers to apply custom titles correctly         

tests/sentry/grouping/init.py

  • Added imports for expand_title_template and get_path utilities
  • Updated _manually_save_event to apply custom title after
    fingerprinting
  • Updated create_event to apply custom title after fingerprinting
  • Added logic to extract and apply custom title template from
    fingerprint info
+20/-1   
fingerprint-title-uses-top-in-app-frame.json
Add fingerprint input for custom title test                           

tests/sentry/grouping/fingerprint_inputs/fingerprint-title-uses-top-in-app-frame.json

  • New test input file with fingerprinting rules that include custom
    title template
  • Defines rule matching do_dog_stuff module with title using {{ function
    }} variable
  • Includes stacktrace with multiple frames to test in-app frame
    selection
+38/-0   
fingerprint_title_uses_top_in_app_frame.pysnap
Add snapshot test for custom title frame selection             

tests/sentry/grouping/snapshots/test_fingerprinting/test_event_hash_variant/fingerprint_title_uses_top_in_app_frame.pysnap

  • New snapshot test validating custom title uses top in-app frame
  • Verifies title resolves to Dogs are great (throw_ball) using in-app
    frame function
  • Confirms custom fingerprint variant contributes and other variants are
    ignored
+44/-0   

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Auditing: The new logic applying custom titles and fingerprint metadata does not emit any audit logs
for these critical grouping mutations, which may be acceptable if logging is handled
elsewhere but is not evident in this diff.

Referred Code
    # Check if the fingerprint includes a custom title, and if so, set the event's title accordingly.
    _apply_custom_title_if_needed(fingerprint_info, event)

    for variant in strategy_component_variants.values():
        variant.root_component.update(contributes=False, hint=hint)

elif fingerprint_type == "hybrid":
    for variant_name, variant in strategy_component_variants.items():
        # Since we're reusing the variant names, when all of the variants are combined, these
        # salted versions will replace the unsalted versions
        additional_variants[variant_name] = SaltedComponentVariant.from_component_variant(
            variant, resolved_fingerprint, fingerprint_info
        )

final_variants = {
    **strategy_component_variants,
    # Add these in second, so the salted versions of any variants replace the unsalted versions
    **additional_variants,
}

# Ensure we have a fallback hash if nothing else works out


 ... (clipped 17 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing Validation: The helper _apply_custom_title_if_needed assumes expand_title_template and event.data are
valid without guarding against malformed templates or missing structures, which may be
safe given existing invariants but is not explicit here.

Referred Code
def _apply_custom_title_if_needed(fingerprint_info: FingerprintInfo, event: Event) -> None:
    """
    If the given event has a custom fingerprint which includes a title template, apply the custom
    title to the event.
    """
    custom_title_template = get_path(fingerprint_info, "matched_rule", "attributes", "title")

    if custom_title_template:
        resolved_title = expand_title_template(custom_title_template, event)
        event.data["title"] = resolved_title

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Template Handling: Test helper applies a custom title by passing fingerprint_info to expand_title_template
instead of the event, which may indicate inconsistent input handling for templates though
it might be intentional for tests.

Referred Code
normalize_stacktraces_for_grouping(data, load_grouping_config(grouping_config))

data.setdefault("fingerprint", ["{{ default }}"])
apply_server_side_fingerprinting(data, fingerprinting_config)
fingerprint_info = data.get("_fingerprint_info", {})
custom_title_template = get_path(fingerprint_info, "matched_rule", "attributes", "title")

# Technically handling custom titles happens during grouping, not before it, but we're not
# running grouping until later, and the title needs to be set before we get metadata below.
if custom_title_template:
    resolved_title = expand_title_template(custom_title_template, fingerprint_info)
    data["title"] = resolved_title

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use correct data for title expansion

In _manually_save_event, pass the event data dictionary to expand_title_template
instead of fingerprint_info to ensure correct title variable resolution.

tests/sentry/grouping/init.py [97-101]

 # Technically handling custom titles happens during grouping, not before it, but we're not
 # running grouping until later, and the title needs to be set before we get metadata below.
 if custom_title_template:
-    resolved_title = expand_title_template(custom_title_template, fingerprint_info)
+    resolved_title = expand_title_template(custom_title_template, data)
     data["title"] = resolved_title
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that expand_title_template needs the full event data (data) to resolve template variables, not just the fingerprint_info. This is a bug in the test helper that could lead to incorrect test outcomes for custom titles.

Medium
Apply custom title in test helper

In FingerprintInput.create_event, add logic to expand the custom title template
and set data["title"], mirroring the implementation in _manually_save_event.

tests/sentry/grouping/init.py [330-337]

 data.setdefault("fingerprint", ["{{ default }}"])
 apply_server_side_fingerprinting(data, config)
 fingerprint_info = data.get("_fingerprint_info", {})
 custom_title_template = get_path(fingerprint_info, "matched_rule", "attributes", "title")
 
+if custom_title_template:
+    resolved_title = expand_title_template(custom_title_template, data)
+    data["title"] = resolved_title
+
 event_type = get_event_type(data)
 event_metadata = event_type.get_metadata(data)
 data.update(materialize_metadata(data, event_type, event_metadata))
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the test helper FingerprintInput.create_event is missing logic to apply custom titles, which is inconsistent with other parts of the test setup. Adding this logic improves test completeness and consistency.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants