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

All exceptions are now added as breadcrumbs on future events #3584

Merged
merged 14 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- All exceptions are now added as breadcrumbs on future events. Previously this was only the case for exceptions captured via the `Sentry.SeriLog` or `Sentry.Extensions.Logging` integrations. ([#3584](https://github.com/getsentry/sentry-dotnet/pull/3584))

### Fixes
- On mobile devices, the SDK no longer throws a `FormatException` for `ProcessorFrequency` when trying to report native events ([#3541](https://github.com/getsentry/sentry-dotnet/pull/3541))

Expand Down
9 changes: 7 additions & 2 deletions src/Sentry.Extensions.Logging/SentryLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,14 @@ public void Log<TState>(
var @event = CreateEvent(logLevel, eventId, state, exception, message, CategoryName);

_ = _hub.CaptureEvent(@event);

// Capturing exception events adds a breadcrumb automatically... we don't want to add another one
if (exception != null)
{
return;
}
}

// Even if it was sent as event, add breadcrumb so next event includes it
if (ShouldAddBreadcrumb(logLevel, eventId, exception))
{
var data = eventId.ToDictionaryOrNull();
Expand All @@ -72,7 +77,7 @@ public void Log<TState>(

_hub.AddBreadcrumb(
_clock,
message ?? exception?.Message!,
(message ?? exception?.Message)!,
CategoryName,
null,
data,
Expand Down
6 changes: 6 additions & 0 deletions src/Sentry.NLog/SentryTarget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,12 @@ private void InnerWrite(LogEventInfo logEvent)
}

_ = hub.CaptureEvent(evt);

// Capturing exception events adds a breadcrumb automatically... we don't want to add another one
if (exception != null)
{
return;
}
}

// Whether or not it was sent as event, add breadcrumb so the next event includes it
Expand Down
7 changes: 6 additions & 1 deletion src/Sentry.Serilog/SentrySink.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,14 @@ private void InnerEmit(LogEvent logEvent)
evt.SetExtras(GetLoggingEventProperties(logEvent));

hub.CaptureEvent(evt);

// Capturing exception events adds a breadcrumb automatically... we don't want to add another one
if (exception != null)
{
return;
}
}

// Even if it was sent as event, add breadcrumb so next event includes it
if (logEvent.Level < _options.MinimumBreadcrumbLevel)
{
return;
Expand Down
57 changes: 52 additions & 5 deletions src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,44 @@ private void ApplyTraceContextToEvent(SentryEvent evt, SentryPropagationContext
evt.DynamicSamplingContext = propagationContext.GetOrCreateDynamicSamplingContext(_options);
}

public bool CaptureEnvelope(Envelope envelope) => CurrentClient.CaptureEnvelope(envelope);

private void AddBreadcrumbForException(SentryEvent evt, Scope scope)
{
try
{
if (!IsEnabled || evt.Exception is not { } exception)
{
return;
}

var exceptionMessage = exception.Message ?? "";
var formatted = evt.Message?.Formatted;

string breadcrumbMessage;
Dictionary<string, string>? data = null;
if (string.IsNullOrWhiteSpace(formatted))
{
breadcrumbMessage = exceptionMessage;
}
else
{
breadcrumbMessage = formatted;
// Exception.Message won't be used as Breadcrumb message
// Avoid losing it by adding as data:
data = new Dictionary<string, string>
{
{"exception_message", exceptionMessage}
};
}
Comment on lines +393 to +411
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double-checking: If I've got my log-level set to Warning and the SDK creates an event out of a log warning:

  • The minimum level is reached and the integration captures an event, skipping the breadcrumb creation
  • From the logging integration we get here
  • There was no exception - we bail early
  • Warnings don't get added as breadcrumbs by neither the logging integration nor the core SDK?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch... have pushed a new commit to fix (the SentryLogger and SentrySink now do create breadcrumbs for events, if no exceptions are associated with those events).

I think that's the correct behaviour since CaptureEvent is also used by CaptureMessage (and I assume we don't want to automatically add breadcrumbs for calls to CaptureMessage):

return hub.CaptureEvent(sentryEvent, configureScope);

scope.AddBreadcrumb(breadcrumbMessage, "Exception", data: data, level: BreadcrumbLevel.Critical);
}
catch (Exception e)
{
_options.LogError(e, "Failure to store breadcrumb for exception event: {0}", evt.EventId);
}
}

public SentryId CaptureEvent(SentryEvent evt, Action<Scope> configureScope)
=> CaptureEvent(evt, null, configureScope);

Expand All @@ -394,7 +432,12 @@ public SentryId CaptureEvent(SentryEvent evt, SentryHint? hint, Action<Scope> co
var clonedScope = CurrentScope.Clone();
configureScope(clonedScope);

return CaptureEvent(evt, clonedScope, hint);
// Although we clone a temporary scope for the configureScope action, for the second scope
// argument (the breadcrumbScope) we pass in the current scope... this is because we want
// a breadcrumb to be left on the current scope for exception events
jamescrosswell marked this conversation as resolved.
Show resolved Hide resolved
var eventId = CaptureEvent(evt, hint, clonedScope);
AddBreadcrumbForException(evt, CurrentScope);
return eventId;
}
catch (Exception e)
{
Expand All @@ -403,9 +446,15 @@ public SentryId CaptureEvent(SentryEvent evt, SentryHint? hint, Action<Scope> co
}
}

public bool CaptureEnvelope(Envelope envelope) => CurrentClient.CaptureEnvelope(envelope);

public SentryId CaptureEvent(SentryEvent evt, Scope? scope = null, SentryHint? hint = null)
{
scope ??= CurrentScope;
var eventId = CaptureEvent(evt, hint, scope);
AddBreadcrumbForException(evt, scope);
return eventId;
}

private SentryId CaptureEvent(SentryEvent evt, SentryHint? hint, Scope scope)
{
if (!IsEnabled)
{
Expand All @@ -414,8 +463,6 @@ public SentryId CaptureEvent(SentryEvent evt, Scope? scope = null, SentryHint? h

try
{
scope ??= CurrentScope;

// We get the span linked to the event or fall back to the current span
var span = GetLinkedSpan(evt) ?? scope.Span;
if (span is not null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@
Id: Guid_1,
IpAddress: {{auto}}
},
Breadcrumbs: [
{
Message: my exception,
Category: Exception,
Level: critical
}
],
Spans: [
{
IsFinished: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@
Id: Guid_1,
IpAddress: {{auto}}
},
Breadcrumbs: [
{
Message: my exception,
Category: Exception,
Level: critical
}
],
Spans: [
{
IsFinished: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@
Id: Guid_1,
IpAddress: {{auto}}
},
Breadcrumbs: [
{
Message: my exception,
Category: Exception,
Level: critical
}
],
Spans: [
{
IsFinished: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@
Id: Guid_1,
IpAddress: {{auto}}
},
Breadcrumbs: [
{
Message: my exception,
Category: Exception,
Level: critical
}
],
Spans: [
{
IsFinished: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@
Id: Guid_1,
IpAddress: {{auto}}
},
Breadcrumbs: [
{
Message: my exception,
Category: Exception,
Level: critical
}
],
Spans: [
{
IsFinished: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@
IpAddress: {{auto}}
},
Environment: production,
Breadcrumbs: [
{
Message: my exception,
Category: Exception,
Level: critical
}
],
Spans: [
{
IsFinished: true,
Expand Down
Loading
Loading