Skip to content

Conversation

@tustanivsky
Copy link
Contributor

This PR introduces stack trace rules specific to Unreal Engine. Having these set by default will allow us to merge getsentry/sentry-unreal#744 and resolve getsentry/sentry-unreal#669.

Essentially, the suggested set of rules addresses grouping issues we encountered with assertion/ensure events. It also takes care of cutting off redundant callstack frames for regular events sent via the UE SDK interface.

@tustanivsky tustanivsky requested a review from a team as a code owner January 22, 2025 08:16
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 22, 2025
@armenzg

This comment was marked as resolved.

@tustanivsky
Copy link
Contributor Author

Hey @armenzg, here are some example events that I've captured in Unreal with the suggested rules applied:

Copy link
Member

@armenzg armenzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the examples!
Could you please add the tests? Let me know if you need a hand adding them and running them.

## Unreal Engine

### Unreal Engine internal assertion handling
family:native stack.function:FDebug::CheckVerifyFailedImpl* ^-app -app v+app ^-group -group v+group
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example of this rule.
image

and this:
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tustanivsky is this change also intentional?
image

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, apparently __start-thread and __pthread_start were un-ignored because of the v+app in this rule. Is there a way we can still keep them ignored?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would have to add specific rules to keep them as non-in-app.

family:native stack.function:FDebug::CheckVerifyFailedImpl* ^-app -app v+app ^-group -group v+group

### Unreal Engine internal ensure handling
family:native stack.function:UE::Assert::Private::ExecCheckImplInternal* ^-app -app v+app ^-group -group v+group
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example of this rule:
image

@tustanivsky there are 5 frames excluded from grouping that have source code (see the ones which can be expanded).

Should any of those be in-app? or part of grouping?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe these frames represent internal Unreal SDK logic for processing ensure events and can be safely ignored.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we ignore all frames with sentry_ in all native crashes? platform=native ?

family:native stack.function:USentrySubsystem::CaptureMessageWithScope* ^-app -app v+app ^-group -group v+group
family:native stack.function:USentrySubsystem::CaptureEvent* ^-app -app v+app ^-group -group v+group
family:native stack.function:USentrySubsystem::CaptureEventWithScope* ^-app -app v+app ^-group -group v+group
family:native stack.function:USentrySubsystem::*execCapture* ^-app -app v+app ^-group -group v+group
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example of this rule:
image

@tustanivsky
Copy link
Contributor Author

Thanks, I'll look into adding some tests and let you know if any questions arise.

@lobsterkatie
Copy link
Member

The test inputs actually be should be in tests/sentry/grouping/grouping_inputs/. Once you add them there, run export SENTRY_SNAPSHOTS_WRITEBACK=1 in your terminal and run tests/sentry/grouping/test_variants.py, tests/sentry/grouping/test_grouphash_metadata.py, and tests/sentry/grouping/test_fingerprinting.py. Check over the snapshots which are generated (note that the legacy ones won't apply your new rules, which is expected) to make sure everything looks right, then commit both the new inputs and the snapshots.

LMK if any of that is unclear.

@armenzg armenzg changed the title Add UE-specific stack trace rules Add Unreal Engine stack trace rules Jan 27, 2025
@armenzg
Copy link
Member

armenzg commented Feb 13, 2025

I will take care of pushing this to production next week.

@armenzg armenzg closed this Feb 13, 2025
@armenzg armenzg mentioned this pull request Feb 18, 2025
armenzg added a commit that referenced this pull request Feb 19, 2025
This will not change system hashes, thus, we can deploy it without a project transition.

This PR introduces stack trace rules specific to Unreal Engine. Having these set by default will allow us to merge getsentry/sentry-unreal#744 and resolve getsentry/sentry-unreal#669.

Essentially, the suggested set of rules addresses grouping issues we encountered with assertion/ensure events. It also takes care of cutting off redundant call stack frames for regular events sent via the UE SDK interface.

Original rules by @tustanivsky .

Supersedes:
* #83813
armenzg added a commit that referenced this pull request Feb 19, 2025
This will not change system hashes, thus, we can deploy it without a project transition.

This PR introduces stack trace rules specific to Unreal Engine. Having these set by default will allow us to merge getsentry/sentry-unreal#744 and resolve getsentry/sentry-unreal#669.

Essentially, the suggested set of rules addresses grouping issues we encountered with assertion/ensure events. It also takes care of cutting off redundant call stack frames for regular events sent via the UE SDK interface.

Original rules by @tustanivsky .

Supersedes:
* #83813
armenzg added a commit that referenced this pull request Feb 20, 2025
This will not change system hashes, thus, we can deploy it without a project transition.

This PR introduces stack trace rules specific to Unreal Engine. Having these set by default will allow us to merge getsentry/sentry-unreal#744 and resolve getsentry/sentry-unreal#669.

Essentially, the suggested set of rules addresses grouping issues we encountered with assertion/ensure events. It also takes care of cutting off redundant call stack frames for regular events sent via the UE SDK interface.

Original rules by @tustanivsky .

Supersedes:
* #83813
armenzg added a commit that referenced this pull request Feb 21, 2025
This will not change system hashes, thus, we can deploy it without a project transition. It's based on #85452.

This PR introduces stack trace rules specific to Unreal Engine. Having these set by default will allow us to merge getsentry/sentry-unreal#744 and resolve getsentry/sentry-unreal#669.

Essentially, the suggested set of rules addresses grouping issues we encountered with assertion/ensure events. It also cuts off redundant call stack frames for regular events sent via the UE SDK interface.

Original rules by @tustanivsky .

Supersedes:
* #83813
Jesse-Box pushed a commit that referenced this pull request Feb 21, 2025
This will not change system hashes, thus, we can deploy it without a project transition. It's based on #85452.

This PR introduces stack trace rules specific to Unreal Engine. Having these set by default will allow us to merge getsentry/sentry-unreal#744 and resolve getsentry/sentry-unreal#669.

Essentially, the suggested set of rules addresses grouping issues we encountered with assertion/ensure events. It also cuts off redundant call stack frames for regular events sent via the UE SDK interface.

Original rules by @tustanivsky .

Supersedes:
* #83813
@github-actions github-actions bot locked and limited conversation to collaborators Mar 1, 2025
@tustanivsky tustanivsky deleted the ue-grouping-rules branch September 8, 2025 05:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Configure stack trace rules for Unreal Engine

4 participants