Skip to content

Capture open transactions on disabled hubs#2319

Merged
mattjohnsonpint merged 5 commits intomainfrom
fix/sdk-init-capture-transaction
Apr 21, 2023
Merged

Capture open transactions on disabled hubs#2319
mattjohnsonpint merged 5 commits intomainfrom
fix/sdk-init-capture-transaction

Conversation

@mattjohnsonpint
Copy link
Contributor

@mattjohnsonpint mattjohnsonpint commented Apr 20, 2023

This is a subtle but important change to how transactions are managed with regard to disabled hubs.

Previously, if the hub was disabled (ie SentrySdk.Init was disposed) then no more transactions could be captured on that hub. This is a problem in the case where an unhandled exception occurs in an async Main (or a top-level async main), because open transactions aren't marked as failed/aborted, but just not captured at all. The corresponding error event then has a Trace ID that can't be correlated to any transaction.

With this change, we now prevent new transactions from being started after a hub is disabled (by ensuring they're sampled out). But we will go ahead and capture any that were already started before the hub was disabled.

This ultimately has the side effect of resolving #321 - fully working correctly with async main. The exception side of that was inadvertently resolved with #2103. This PR takes care of the associated transaction.

We can also now change our position on disposing from SentrySdk.Init. In most cases you no longer need to dispose it, because we will flush automatically in AppDomainProcessExitIntegration. The reasons to still dispose are: if you have disabled that integration, or if you want to do unmonitored work after disabling Sentry, or if you are writing an integration that has its own lifetime events. Otherwise, disposing doesn't hurt, but it is no longer required. Comments are adjusted accordingly.

We also had some strong wording about users not calling CaptureTransaction directly, so I've hidden those with [EditorBrowsable(EditorBrowsableState.Never)] attributes.

@mattjohnsonpint
Copy link
Contributor Author

Note: After this is released, we can update troubleshooting docs at https://docs.sentry.io/platforms/dotnet/troubleshooting/#unhandled-exceptions-are-not-captured-when-using-an-async-main-method to just say to update the SDK version.

@mattjohnsonpint
Copy link
Contributor Author

Docs pr: getsentry/sentry-docs#6707

@mattjohnsonpint mattjohnsonpint merged commit c6a06fd into main Apr 21, 2023
@mattjohnsonpint mattjohnsonpint deleted the fix/sdk-init-capture-transaction branch April 21, 2023 06:50
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