-
-
Notifications
You must be signed in to change notification settings - Fork 53
Add callback to pre-process breadcrumbs before adding #814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
38928b1
Add beforeBreadcrumb callback stubs
tustanivsky f20ad5b
Add hook implementation for Apple
tustanivsky f111c5a
Fix threading issues on Android
tustanivsky 8741a22
Fix potential recursive onBeforeBreadcrumb handler invocation when pr…
tustanivsky f979d22
Update changelog
tustanivsky 553a695
Update package snapshot
tustanivsky 8f1cbff
Remove unimplemented api
tustanivsky aad68c3
Merge branch 'main' into feat/breadcrumb-hook
tustanivsky 16b43b0
Replace game thread check with static init check
tustanivsky 005bd1d
Updated GC checks
tustanivsky d451940
Merge branch 'main' into feat/breadcrumb-hook
tustanivsky 64392b6
Update package snapshots
tustanivsky b9ac8e5
Fix beforeBreadcrumb for native
tustanivsky 3f880a5
Fix minor error/warnings
tustanivsky abde1d0
Remove static init check
tustanivsky e744eea
Update readme
tustanivsky 239f60b
Add beforeBreadcrumbHandler to sample
tustanivsky 29df7a7
Fix build error on Mac
tustanivsky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 changes: 11 additions & 0 deletions
11
plugin-dev/Source/Sentry/Private/SentryBeforeBreadcrumbHandler.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| // Copyright (c) 2023 Sentry. All Rights Reserved. | ||
|
|
||
| #include "SentryBeforeBreadcrumbHandler.h" | ||
|
|
||
| #include "SentryBreadcrumb.h" | ||
| #include "SentryHint.h" | ||
|
|
||
| USentryBreadcrumb* USentryBeforeBreadcrumbHandler::HandleBeforeBreadcrumb_Implementation(USentryBreadcrumb* Breadcrumb, USentryHint* Hint) | ||
| { | ||
| return Breadcrumb; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
running on the main thread will be quite common. Why is it not allowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes issues when wrapping Android
jobjectinstances in Unreal sinceUSentryBreadcrumbandUSentryHint(both derived fromUObject) can only be safely created on theGameThread.Despite certain breadcrumb types (i.e. Android activity and app lifecycle callbacks) originating from the
mainthread and lacking pre-processing support they will still be included in captured events.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the behavior a user can expect? some crumbs will go through the callback and some won't?
it just seems like a surprising behavior.
"why can't I drop these breadcrumbs through
beforeBreadcrumb?"Should we avoid adding a callback altogether then?
Or how do we avoid surprises/disapointments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bruno-garcia The original error message I've seen while testing this led me to believe that calling the
beforeBreadcrumbhook outside the game thread on Android was the issue but it turned out to be misleading. Similar tobeforeSend(#860) the real limitation is that we can't call it during GC otherwise it works as expected.I've added this limitation to the README and will also ensure it's mentioned in the documentation as @bitsandfoxes suggested.