-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
Add Hint support #2351
Add Hint support #2351
Conversation
- Marked SentryOptions.BeforeSend property obsolete - Added Hint class and Tests to be used as a parameter to SetBeforeSend
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Add Hint support ([#2351](https://github.com/getsentry/sentry-dotnet/pull/2351)) If none of the above apply, you can opt out of this check by adding |
With respect to
The |
- Added missing XML docs on SentryOptions
…readcrumbs Added hint support to SentrySdk.AddBreadcrumb Fixed StackOverflow exception calling HubExtensions.CaptureEventInternal
…rows_ErrorToEventBreadcrumb
Not able to reproduce CI Build errors on my local machine... trying one more thing before potentially recommending we remove this test. This code change should remove an extra line from the call stack.
I fixed the iOS build issue. The Cocoa SDK doesn't have hints yet, and as you pointed out in comments, the Android SDK doesn't really let us propagate hints. But this is fine for now. We can address later with some coordination with the other teams if we think it will add value. |
We need to do the same with But also we need to supply hints for event and transaction processors (not exception processors). I was thinking that we should use default interface methods for this, but I think a cleaner approach is to implement abstract classes instead. Specifically: public abstract class SentryEventProcessor : ISentryEventProcessor
{
SentryEvent ISentryEventProcessor.Process(SentryEvent @event)
{
throw new NotImplementedException();
}
public abstract SentryEvent? Process(SentryEvent @event, Hint hint);
}
public abstract class SentryTransactionProcessor : ISentryTransactionProcessor
{
Transaction ISentryTransactionProcessor.Process(Transaction transaction)
{
throw new NotImplementedException();
}
public abstract Transaction? Process(Transaction transaction, Hint hint);
} So if one wants hints in processors, they implement the abstract class. We can remove the current interfaces in the next major release (v4.0.0). Where we have usages of The alternative would be to provide a completely separate |
…rks, capturing Events or sending Transactions
- Added overload to CaptureEvent to allow passing both Hint and ConfigureScope callback
I think I've taken this as far as I can. Let me know when you get a chance to review... and @mattjohnsonpint would be great to get your wisdom on the CI Build error since the most recent merge from |
Adds new APIs that provide callbacks containing
Hint
object:SentryOptions.SetBeforeSend
SentryOptions.SetBeforeSendTransaction
SentryOptions.SetBeforeBreadcrumb
ISentryEventProcessorWithHint
ISentryTransactionProcessorWithHint
NOTE: Previously the first three of those were properties:
BeforeSend
,BeforeSendTransaction
,BeforeBreadcrumb
. These properties have been obsoleted in favor of methods, so that they can be overloaded when hints are desired.Hints provide two specific functions:
Hint
support to beforeSend and Event processors #1469)