sentry-dotnet icon indicating copy to clipboard operation
sentry-dotnet copied to clipboard

Feedback captures are subject to sample rate

Open codecat opened this issue 1 year ago • 6 comments

Package

Sentry

.NET Flavor

.NET

.NET Version

8.0.8

OS

Windows

SDK Version

4.9.0

Self-Hosted Sentry Version

No response

Steps to Reproduce

Initialize Sentry using:

SentrySdk.Init(o => {
  o.Dsn = "...";
  o.SampleRate = 0.2f;
});

Then capture multiple feedback items, as per the documentation:

for (int i = 0; i < 6; i++) {
  var eventId = SentrySdk.CaptureMessage("An event that will receive user feedback.");
  SentrySdk.CaptureUserFeedback(eventId, "[email protected]", $"Test {i}", "The User");
}

(You can also do these manually multiple times instead of in a for loop, which is what we actually tested.)

Expected Result

We would expect all points of feedback to not be subject to the sample rate, as described here.

Actual Result

We are only seeing some of the feedback points come through. (And one duplicate one w/o a linked issue in the below screenshot, strangely enough.)

Image

As a workaround, we tested re-initializing the Sentry SDK with a sampling rate of 100% prior to users submitting feedback, which works. This is not an ideal scenario, so I hope we can see this fixed.

codecat avatar Sep 10 '24 06:09 codecat

Hi @codecat,

Thanks for getting in touch. I can't see logically how this might be happening though. The API you're calling simply delegates to the Hub: https://github.com/getsentry/sentry-dotnet/blob/76e9f660296dd23ff5d1f63e8c3f55e9815c8e6d/src/Sentry/SentrySdk.cs#L501-L502

And the Hub in turn passes the feedback on to the SentryClient: https://github.com/getsentry/sentry-dotnet/blob/5ff8acc6710136297ae436967b081278a10a4eae/src/Sentry/Internal/Hub.cs#L456-L471

And the SentryClient simply turns the feedback into an envelope and enqueues this for delivery: https://github.com/getsentry/sentry-dotnet/blob/5b35d8ea901225019929ff93bf7b24720f604d95/src/Sentry/SentryClient.cs#L85-L95

So, at no stage, is any rate limiting implemented by the SDK for User Feedback.

The only thing I can think is that maybe the sentry server is dropping those events for some reason. In that case, you might see some record of that in the "Stats" tab in your Sentry portal.

jamescrosswell avatar Sep 10 '24 08:09 jamescrosswell

Thanks for looking into this.

My initial thought was that it might be related to the attached message (using CaptureMessage) being affected by sampling, resulting in CaptureUserFeedback failing. For example, I also can't call it with SentryId.Empty (it will silently fail).

On the Stats page, I see Filtered, Rate Limited, and Invalid all at 0.

codecat avatar Sep 10 '24 08:09 codecat

After adding some logging I can confirm this is indeed what is happening:

2024-09-10 11:10:04.6697|DEBUG|SentryLogger|Event sampled.
2024-09-10 11:10:04.6697|WARN|SentryLogger|User feedback dropped due to empty id.

codecat avatar Sep 10 '24 09:09 codecat

2024-09-10 11:10:04.6697|WARN|SentryLogger|User feedback dropped due to empty id.

Aha, OK that makes sense. I assume User Feedback is designed to be supplied with Exception Events... so after capturing an exception you could then capture some User Feedback and submit this (along with the id of the Exception that was captured).

Are you trying to achieve something different?

jamescrosswell avatar Sep 10 '24 12:09 jamescrosswell

We have a special Feedback dialog that users can open on demand (through a button on our UI) that the user can choose to optionally attach logs to. It looks like this:

Image

In addition, we have some other parts of our UI which don't necessarily generate an exception, but do generate user feedback (eg. in our case, a user receives a warning from us about something, but they think it's a false positive, so they click on a button to indicate as such), which we really do want to hear about.

Are we misunderstanding the use case of Sentry's User Feedback, or is this indeed a problem/bug in the .Net SDK?

codecat avatar Sep 10 '24 12:09 codecat

Are we misunderstanding the use case of Sentry's User Feedback, or is this indeed a problem/bug in the .Net SDK?

User feedback was implemented before I started working on the SDK so I'm learning here at the same time as you, but it looks like initially User Feedback events did have to be associated with events and so the code in the SDK is doing what it's supposed to. However, since then, a new format has been introduced that allows User Feedback to be sent independently of events... and presumably this newer User Feedback mechanism hasn't yet been implemented in the .NET SDK.

So I think you've understood the intent of the new User Feedback model but the .NET SDK is still built using the old APIs.

I've raised the following issue for prioritisation:

  • https://github.com/getsentry/sentry-dotnet/issues/3605

cc: @bitsandfoxes

jamescrosswell avatar Sep 10 '24 21:09 jamescrosswell

@jamescrosswell this could be a good next project.

And getting a widget for MAUI would be nice too. Based on the work done by Andrew on iOS, or Denis on Flutter. I expanded the feature details to have relevant links and pointer:

  • https://github.com/getsentry/sentry-dotnet/issues/3605#issue-2517940817

bruno-garcia avatar Nov 20 '24 17:11 bruno-garcia

@jamescrosswell Thoughts?

Proposal

  • SentrySDK.IsTransactionLimitExceeded
  • SentrySDK.IsQuotaExceeded
if (SentrySDK.IsTransactionLimitExceeded && !SentrySDK.IsQuotaExceeded) {
   // collect user feedback
   // send user feedback like normal
}

aritchie avatar Feb 13 '25 21:02 aritchie

@jamescrosswell Thoughts?

Probably not for this issue (we need to implement User Feedback using the new APIs to address this issue).

That's more relevant for https://github.com/getsentry/sentry-dotnet/issues/3947 but it's done automatically by our transport, so SDK users doesn't need to worry/think about this.

jamescrosswell avatar Feb 13 '25 22:02 jamescrosswell