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

Add LogCat integration to Android via an EventProcessor #2926

Merged
merged 28 commits into from
Dec 2, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9b46bac
Add LogCat integration for Android
kanadaj Nov 29, 2023
e3dccd5
Add better comments and disable the feature on Android 23, though rea…
kanadaj Nov 29, 2023
27bc903
Allow collecting logcat for all events, not just errors
kanadaj Nov 29, 2023
cd12536
Update comment
kanadaj Nov 29, 2023
96b4a27
Update comment
kanadaj Nov 29, 2023
2eb5749
Add change to CHANGELOG.md
kanadaj Nov 29, 2023
a6b3778
Fix namespace imports
kanadaj Nov 29, 2023
c6a1872
Fix Android import
kanadaj Nov 29, 2023
0a28c76
Add LogCatIntegration bindable option
kanadaj Nov 29, 2023
1f1c345
Move the LogCat integration to Sentry/Platofrms/Android
kanadaj Nov 30, 2023
b8670d6
Update CHANGELOG.md
kanadaj Nov 30, 2023
da2f601
Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs
kanadaj Nov 30, 2023
c686129
Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs
kanadaj Nov 30, 2023
c9ecd37
Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs
kanadaj Nov 30, 2023
1ea9a95
Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs
kanadaj Nov 30, 2023
abcde5a
Fix SentrySdk Android settings
kanadaj Nov 30, 2023
658edcc
Update CHANGELOG.md
kanadaj Dec 1, 2023
3bce305
Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs
kanadaj Dec 1, 2023
3e9aaba
Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs
kanadaj Dec 1, 2023
98a6cb2
Add LogCatMaxLines option, disable event processor if we can't access…
kanadaj Dec 1, 2023
940df40
Use MemoryStream instead of output file
kanadaj Dec 1, 2023
0988465
Remove unused imports
kanadaj Dec 1, 2023
55ec89c
Add logcat integration to the samples
kanadaj Dec 1, 2023
ff8ac25
Update CHANGELOG.md
kanadaj Dec 1, 2023
c9f99e8
Update src/Sentry/Platforms/Android/SentryOptions.cs
kanadaj Dec 1, 2023
d76d0ed
Update src/Sentry/Platforms/Android/SentryOptions.cs
kanadaj Dec 1, 2023
e68543b
Disable integration by default
kanadaj Dec 1, 2023
a229633
Add LogCatMaxLines to BindableSentryOptions.cs
kanadaj Dec 1, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Features

- Native crash reporting on NativeAOT published apps (Windows, Linux, macOS). ([#2887](https://github.com/getsentry/sentry-dotnet/pull/2887))
- Added Android LogCat collection to the core `Sentry` SDK, configurable via `SentryOptions.LogCatIntegration` when targeting `net7.0-android` or later. ([#2926](https://github.com/getsentry/sentry-dotnet/pull/2926))
kanadaj marked this conversation as resolved.
Show resolved Hide resolved

### API breaking Changes

Expand Down
4 changes: 4 additions & 0 deletions src/Sentry/Platforms/Android/BindableSentryOptions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// ReSharper disable once CheckNamespace
using Sentry.Android;

namespace Sentry;

internal partial class BindableSentryOptions
Expand Down Expand Up @@ -33,6 +35,7 @@ public class AndroidOptions
public TimeSpan? ReadTimeout { get; set; }
public bool? EnableAndroidSdkTracing { get; set; }
public bool? EnableAndroidSdkBeforeSend { get; set; }
public LogCatIntegrationType? LogCatIntegration { get; set; }

public void ApplyTo(SentryOptions.AndroidOptions options)
{
Expand All @@ -59,6 +62,7 @@ public void ApplyTo(SentryOptions.AndroidOptions options)
options.ReadTimeout = ReadTimeout ?? options.ReadTimeout;
options.EnableAndroidSdkTracing = EnableAndroidSdkTracing ?? options.EnableAndroidSdkTracing;
options.EnableAndroidSdkBeforeSend = EnableAndroidSdkBeforeSend ?? options.EnableAndroidSdkBeforeSend;
options.LogCatIntegration = LogCatIntegration ?? options.LogCatIntegration;
}
}
}
96 changes: 96 additions & 0 deletions src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
using Sentry.Extensibility;
using Runtime = Java.Lang.Runtime;
using Android.Content;
using Application = Android.App.Application;

namespace Sentry.Android;

internal class LogCatAttachmentEventProcessor : ISentryEventProcessorWithHint
{
private static bool SendLogcatLogs = true;
private readonly LogCatIntegrationType _logCatIntegrationType;
private readonly IDiagnosticLogger? _diagnosticLogger;

public LogCatAttachmentEventProcessor(LogCatIntegrationType logCatIntegrationType, IDiagnosticLogger? diagnosticLogger)
{
_logCatIntegrationType = logCatIntegrationType;
_diagnosticLogger = diagnosticLogger;
}

public SentryEvent Process(SentryEvent @event, Hint hint)
{
// If sending has failed once, we have to disable this feature to prevent infinite loops and to allow the SDK to work otherwise
if (!SendLogcatLogs)
{
return @event;
}

// The logcat command only works on Android API 23 and above
if (!OperatingSystem.IsAndroidVersionAtLeast(23))
{
SendLogcatLogs = false;
return @event;
}

try
{
if (_logCatIntegrationType != LogCatIntegrationType.All && (@event.SentryExceptions?.Any() ?? false))
kanadaj marked this conversation as resolved.
Show resolved Hide resolved
{
return @event;
}

var filesDir = Application.Context.FilesDir;
if (filesDir == null)
{
_diagnosticLogger?.LogWarning("Failed to get files directory");
kanadaj marked this conversation as resolved.
Show resolved Hide resolved
kanadaj marked this conversation as resolved.
Show resolved Hide resolved
return @event;
}

// Only send logcat logs if the event is unhandled if the integration is set to Unhandled
if (_logCatIntegrationType == LogCatIntegrationType.Unhandled)
{
if (!@event.HasTerminalException())
{
return @event;
}
}

// We run the logcat command via the runtime
var process = Runtime.GetRuntime()?.Exec("logcat -t 1000 *:I");
kanadaj marked this conversation as resolved.
Show resolved Hide resolved

// Strangely enough, process.InputStream is the *output* of the command
if (process?.InputStream is null)
{
return @event;
}

// We write the logcat logs to a file so we can attach it
using var output = Application.Context.OpenFileOutput("sentry_logcat.txt", FileCreationMode.Private);
kanadaj marked this conversation as resolved.
Show resolved Hide resolved
if (output is null)
{
return @event;
}

process.InputStream.CopyTo(output);
process.WaitFor();

hint.AddAttachment(filesDir!.Path + "/sentry_logcat.txt", AttachmentType.Default, "text/logcat");
Copy link
Member

Choose a reason for hiding this comment

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

@matejminar we're making up a new mime/type here, is that OK? The goal is to have a customer preview in Sentry that understand logcat and is able to filter by the different columns or highlight stuff etc, wdty?

@romtsn would we want that on Android in general?

Copy link
Member

Choose a reason for hiding this comment

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

@bruno-garcia yes, you'll need to add logcat into this switch to map it to an existing viewer:
https://github.com/getsentry/sentry/blob/d4f5584da02f71f4c2a1a00792b9558c22df61db/static/app/components/events/eventAttachments.tsx#L38-L56

if you want a fancy logcat-specific viewer, you can take an inspo from one of the existing ones there

Copy link
Member

Choose a reason for hiding this comment

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

@bruno-garcia yeah, so far we opted for bytecode manipulation and sending breadcrumbs - this works for the app and libraries logcat calls, but not the system ones. Also it supports all Android versions, while this approach is from API 23 and above.

But generally, I think this would be nice to have, especially when we bump min sdk to 21 or 24 in the future. Should probably be opt-in by default though

Copy link
Member

Choose a reason for hiding this comment

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

Awesome folks! thanks a lot

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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


return @event;
}
catch (Exception e) // Catch all exceptions to prevent crashing the app during logging
{
// Disable the feature if it fails once
SendLogcatLogs = false;

// Log the failure to Sentry
_diagnosticLogger?.LogError(e, "Failed to send logcat logs");
return @event;
}
}

public SentryEvent Process(SentryEvent @event)
{
return Process(@event, new Hint());
bruno-garcia marked this conversation as resolved.
Show resolved Hide resolved
}
}
28 changes: 28 additions & 0 deletions src/Sentry/Platforms/Android/LogCatIntegrationType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
namespace Sentry.Android;

/// <summary>
/// Configures when to attach LogCat logs to events.
/// </summary>
public enum LogCatIntegrationType
{
/// <summary>
/// The LogCat integration is disabled.
/// </summary>
None,
/// <summary>
/// LogCat logs are attached to events only when the event is unhandled.
/// </summary>
Unhandled,

/// <summary>
/// LogCat logs are attached to events with an exception.
/// </summary>
Errors,

/// <summary>
/// LogCat logs are attached to all events.
/// Use caution when enabling, as this may result in a lot of data being sent to Sentry
/// and performance issues if the SDK generates a lot of events.
/// </summary>
All
}
8 changes: 8 additions & 0 deletions src/Sentry/Platforms/Android/SentryOptions.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using Sentry.Android;

// ReSharper disable once CheckNamespace
namespace Sentry;

Expand Down Expand Up @@ -196,6 +198,12 @@ internal AndroidOptions(SentryOptions options)
/// </summary>
public TimeSpan ReadTimeout { get; set; } = TimeSpan.FromSeconds(5);

/// <summary>
/// Gets or sets whether when LogCat logs are attached to events.
/// The default is <see cref="LogCatIntegrationType.Unhandled"/>
/// </summary>
public LogCatIntegrationType LogCatIntegration { get; set; } = LogCatIntegrationType.Unhandled;
kanadaj marked this conversation as resolved.
Show resolved Hide resolved


// ---------- Other ----------

Expand Down
5 changes: 5 additions & 0 deletions src/Sentry/Platforms/Android/SentrySdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Sentry.Android;
using Sentry.Android.Callbacks;
using Sentry.Android.Extensions;
using Sentry.Internal;
using Sentry.JavaSdk.Android.Core;

// Don't let the Sentry Android SDK auto-init, as we do that manually in SentrySdk.Init
Expand Down Expand Up @@ -187,6 +188,10 @@ private static void InitSentryAndroidSdk(SentryOptions options)

// Set options for the managed SDK that depend on the Android SDK. (The user will not be able to modify these.)
options.AddEventProcessor(new AndroidEventProcessor(androidOptions!));
if (options.Android.LogCatIntegration != LogCatIntegrationType.None)
{
options.AddEventProcessor(new LogCatAttachmentEventProcessor(options.Android.LogCatIntegration, options.DiagnosticLogger));
}
options.CrashedLastRun = () => JavaSdk.Sentry.IsCrashedLastRun()?.BooleanValue() is true;
options.EnableScopeSync = true;
options.ScopeObserver = new AndroidScopeObserver(options);
Expand Down
Loading