Skip to content
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

fix: collect main thread data synchronously during init #744

Merged
merged 2 commits into from
May 16, 2022

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented May 12, 2022

After adding the exception handling to SentryInitialization.cs, I've noticed it doesn't collect any of the usual context. This fixes that (and maybe a few other very early user-space error captures). The original behaviour was to run the main thread info collection asynchronously in a coroutine. Now, we collect it right away during init.

Before

https://sentry.io/organizations/sentry-sdks/issues/3267829133/events/2651b13a859b4859860b3f44859ffacd/?project=5439417

image

After

https://sentry.io/organizations/sentry-sdks/issues/3267829133/events/e12e6634035b4eb0bdc3c4349a5d03e1/?project=5439417

image

@bitsandfoxes
Copy link
Contributor

bitsandfoxes commented May 12, 2022

The original reason for the callback solution: #233 (comment)
Can we measure the actual impact once?
@bruno-garcia any thoughts?

@vaind
Copy link
Collaborator Author

vaind commented May 12, 2022

Can we measure the actual impact once?

I've done a rough measurement with the stopwatch (haven't done this in c# before so didn't look up if there's a better tool). In the editor, the runtime of CollectData(), after removing the yields, ~0.002 sec, i.e. 2 milliseconds, but that's an unoptimized build. In a desktop build, it was immeasurable (0.000) without adding an artificial loop. I could run it on mobile, though from the actual APIs we use, I doubt there would be much of a difference.

@vaind vaind merged commit 279acb4 into main May 16, 2022
@vaind vaind deleted the fix/collect-unity-info-earlier branch May 16, 2022 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants