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

Align SDK with docs regarding session update for dropped events

Open adinauer opened this issue 2 years ago • 1 comments

Problem Statement

We've updated the docs (https://github.com/getsentry/develop/pull/551) to explain how we would like to handle event filter order, updating of sessions for dropped events and sending of session updates. We should check if we're aligned and adapt if necessary. Here's the issue from the Java SDK where all of this started: https://github.com/getsentry/sentry-java/issues/1916

Solution Brainstorm

See https://github.com/getsentry/develop/pull/551

adinauer avatar Apr 26 '22 12:04 adinauer

https://develop.sentry.dev/sdk/sessions/#filter-order

mattjohnsonpint avatar Jun 01 '22 21:06 mattjohnsonpint

What this really needs to check is that in all paths, if an unhandled exception causes the session to become unhealthy, that we always update the session. If, for example, we set SentryOptions.SampleRate = 0.0001 and the sessions was unhealthy, we'd still want to send a session update.

mattjohnsonpint avatar Oct 20 '22 22:10 mattjohnsonpint

https://develop.sentry.dev/sdk/sessions/#session-update-filtering

https://develop.sentry.dev/sdk/sessions/#filter-order

mattjohnsonpint avatar Feb 27 '23 22:02 mattjohnsonpint

Most this happens in SentryClient.DoSendEvent in the .NET SDK.

We don't currently do things in the same order as Python then.

Python Order

  1. Check for ignored exception types (a.k.a ignore_errors)
  2. Apply scoped event_processor (a.k.a error_processor)
  3. Apply global event_processor
  4. Apply before_send
  5. Update the session if an event made it this far
  6. Apply sampling rate

Current order in .NET:

  1. Apply sampling rate
  2. Check for ignored exception types (a.k.a ignore_errors) - ApplyExceptionFilters in .NET
  3. Apply scoped event_processor (a.k.a error_processor)
  4. Apply global event_processor
  5. Apply before_send Update the session if an event made it this far ???

Questions

I'm not sure what is meant by "Update the session if an event made it this far" in the issue description.

In .NET, if an event has an error then we end the session or increment the error count in Hub.CaptureEventInternal, just before SentryClient.DoSendEvent gets called:

var hasTerminalException = evt.HasTerminalException();
if (hasTerminalException)
{
    // Event contains a terminal exception -> end session as crashed
    _options.LogDebug("Ending session as Crashed, due to unhandled exception.");
    actualScope.SessionUpdate = _sessionManager.EndSession(SessionEndStatus.Crashed);
}
else if (evt.HasException())
{
    // Event contains a non-terminal exception -> report error
    // (this might return null if the session has already reported errors before)
    actualScope.SessionUpdate = _sessionManager.ReportError();
}

Is that "updating the session"? I'm not sure.

The SessionUpdate created above gets included in the Envelope along with the Event when the Event gets sent after all of the other stuff in SentryClient.DoSendEvent... so that's when it gets sent to Sentry. Is that "updating the session"?

@bruno-garcia or @bitsandfoxes it would be good to get your steer here.

btw: I've moved the sampling decision after all the exception/event processors already (see the related PR).

jamescrosswell avatar Jul 18 '23 09:07 jamescrosswell