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

Feedback captures are subject to sample rate #3604

Open
codecat opened this issue Sep 10, 2024 · 6 comments
Open

Feedback captures are subject to sample rate #3604

codecat opened this issue Sep 10, 2024 · 6 comments

Comments

@codecat
Copy link

codecat commented Sep 10, 2024

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.

@jamescrosswell
Copy link
Collaborator

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:

public static void CaptureUserFeedback(SentryId eventId, string email, string comments, string? name = null)
=> CurrentHub.CaptureUserFeedback(new UserFeedback(eventId, name, email, comments));

And the Hub in turn passes the feedback on to the SentryClient:

public void CaptureUserFeedback(UserFeedback userFeedback)
{
if (!IsEnabled)
{
return;
}
try
{
CurrentClient.CaptureUserFeedback(userFeedback);
}
catch (Exception e)
{
_options.LogError(e, "Failure to capture user feedback: {0}", userFeedback.EventId);
}
}

And the SentryClient simply turns the feedback into an envelope and enqueues this for delivery:

public void CaptureUserFeedback(UserFeedback userFeedback)
{
if (userFeedback.EventId.Equals(SentryId.Empty))
{
// Ignore the user feedback if EventId is empty
_options.LogWarning("User feedback dropped due to empty id.");
return;
}
CaptureEnvelope(Envelope.FromUserFeedback(userFeedback));
}

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.

@codecat
Copy link
Author

codecat commented Sep 10, 2024

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
Copy link
Author

codecat commented Sep 10, 2024

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.

@jamescrosswell
Copy link
Collaborator

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?

@codecat
Copy link
Author

codecat commented Sep 10, 2024

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?

@jamescrosswell
Copy link
Collaborator

jamescrosswell commented Sep 10, 2024

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:

cc: @bitsandfoxes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Status: No status
Development

No branches or pull requests

3 participants