Add LogCat integration to Android via an EventProcessor #2926

kanadaj · 2023-11-29T16:16:30Z

It is possible to fetch LogCat logs using Runtime.Exec() per the following comment: getsentry/sentry-java#1620 (comment)

This SentryEventProcessor automatically grabs the LogCat logs on error (configurable to only log on unhandled errors, all errors or all events) and attaches it to the SentryEvent as a file attachment.

…listically MAUI can't target version 23 anway

bruno-garcia

Thanks a lot for adding this!

CHANGELOG.md

src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

src/Sentry/Platforms/Android/SentrySdk.cs

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

bruno-garcia

I think it looks great!

Could you add this to the sample, please?
We at least should get some manual testing done there and then integration tests (no point in unit testing this I guess?)

CHANGELOG.md

src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

bruno-garcia · 2023-12-01T00:36:15Z

src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

+            process.InputStream.CopyTo(output);
+            process.WaitFor();
+
+            hint.AddAttachment(filesDir!.Path + "/sentry_logcat.txt", AttachmentType.Default, "text/logcat");


@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?

@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

@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

Awesome folks! thanks a lot

getsentry/sentry#61005

getsentry/sentry-java#3075

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

… the FilesDir, generate the file name dynamically

kanadaj · 2023-12-01T00:57:49Z

I think it looks great!

Could you add this to the sample, please? We at least should get some manual testing done there and then integration tests (no point in unit testing this I guess?)

Yeah there is really nothing to unit test, you need a running Android device or emulator with logcat to test this one.

bruno-garcia

This is awesome! Thanks Daniel!

CHANGELOG.md

src/Sentry/Platforms/Android/SentryOptions.cs

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

bruno-garcia · 2023-12-01T01:32:19Z

PR adding it to Android: getsentry/sentry-java#2620

kanadaj · 2023-12-01T03:21:05Z

PR adding it to Android: getsentry/sentry-java#2620

That one uses breadcrumbs though... And while that's possible, imho logcat outputs way too much noise for breadcrumbs to be particularly useful (sometimes you need several hundred log messages from logcat to find what you need).

Afaik it also relies on using Gradle to replace Log.{} calls with Sentry's own thing, and we don't have Gradle here...

bitsandfoxes · 2023-12-01T16:14:39Z

Thanks again for adding this! Lemme figure out what's up with the device tests..

bruno-garcia · 2023-12-01T18:10:06Z

PR adding it to Android: getsentry/sentry-java#2620

That one uses breadcrumbs though... And while that's possible, imho logcat outputs way too much noise for breadcrumbs to be particularly useful (sometimes you need several hundred log messages from logcat to find what you need).

Afaik it also relies on using Gradle to replace Log.{} calls with Sentry's own thing, and we don't have Gradle here...

Some internal chatter came to the conclusion we should:

Have this feature be opt-in
a. potential PII leakage due to apps dumping private stuff
b. potential overhead
c. logcat isn't popular in prod for 'reasons' and that's why Timber became popular, it can be disabled anyway in prod
I think that's all actually, lets have 'false' as default and we should be good to go!

kanadaj · 2023-12-01T19:35:26Z

Sure, lemme do that. ~~Test should also work now.~~ Or not. What the heck?

E1201 19:46:03.030494    3881 FrameBuffer.cpp:3765] Failed to find ColorBuffer:62

kanadaj added 5 commits November 29, 2023 16:06

Add LogCat integration for Android

9b46bac

Add better comments and disable the feature on Android 23, though rea…

e3dccd5

…listically MAUI can't target version 23 anway

Allow collecting logcat for all events, not just errors

27bc903

Update comment

cd12536

Update comment

96b4a27

kanadaj requested review from bitsandfoxes and jamescrosswell as code owners November 29, 2023 16:16

kanadaj added 5 commits November 29, 2023 16:19

Add change to CHANGELOG.md

2eb5749

Fix namespace imports

a6b3778

Fix Android import

c6a1872

Add LogCatIntegration bindable option

0a28c76

Move the LogCat integration to Sentry/Platofrms/Android

1f1c345

bruno-garcia reviewed Nov 30, 2023

View reviewed changes

kanadaj and others added 6 commits November 30, 2023 18:28

Update CHANGELOG.md

b8670d6

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

da2f601

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

c686129

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

c9ecd37

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

1ea9a95

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Fix SentrySdk Android settings

abcde5a

bruno-garcia reviewed Dec 1, 2023

View reviewed changes

kanadaj and others added 6 commits December 1, 2023 00:43

Update CHANGELOG.md

658edcc

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

3bce305

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Update src/Sentry/Platforms/Android/LogCatAttachmentEventProcessor.cs

3e9aaba

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Add LogCatMaxLines option, disable event processor if we can't access…

98a6cb2

… the FilesDir, generate the file name dynamically

Use MemoryStream instead of output file

940df40

Remove unused imports

0988465

Add logcat integration to the samples

55ec89c

bruno-garcia approved these changes Dec 1, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

src/Sentry/Platforms/Android/SentryOptions.cs Show resolved Hide resolved

src/Sentry/Platforms/Android/SentryOptions.cs Outdated Show resolved Hide resolved

Update CHANGELOG.md

ff8ac25

kanadaj and others added 2 commits December 1, 2023 01:11

Update src/Sentry/Platforms/Android/SentryOptions.cs

c9f99e8

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

Update src/Sentry/Platforms/Android/SentryOptions.cs

d76d0ed

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

kanadaj added 2 commits December 1, 2023 19:38

Disable integration by default

e68543b

Add LogCatMaxLines to BindableSentryOptions.cs

a229633

This was referenced Dec 1, 2023

[Event Detail]: Attachment preview for Android logcat output getsentry/sentry#61005

Open

Capture logcat output as attachment getsentry/sentry-java#3075

Open

bruno-garcia merged commit 88bfc29 into getsentry:main Dec 2, 2023
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LogCat integration to Android via an EventProcessor #2926

Add LogCat integration to Android via an EventProcessor #2926

kanadaj commented Nov 29, 2023

bruno-garcia left a comment

bruno-garcia left a comment

bruno-garcia Dec 1, 2023

matejminar Dec 1, 2023

romtsn Dec 1, 2023

bruno-garcia Dec 1, 2023

bruno-garcia Dec 1, 2023

bruno-garcia Dec 1, 2023

kanadaj commented Dec 1, 2023

bruno-garcia left a comment

bruno-garcia commented Dec 1, 2023

kanadaj commented Dec 1, 2023

bitsandfoxes commented Dec 1, 2023 •

edited

Loading

bruno-garcia commented Dec 1, 2023

kanadaj commented Dec 1, 2023 •

edited

Loading

Add LogCat integration to Android via an EventProcessor #2926

Add LogCat integration to Android via an EventProcessor #2926

Conversation

kanadaj commented Nov 29, 2023

bruno-garcia left a comment

Choose a reason for hiding this comment

bruno-garcia left a comment

Choose a reason for hiding this comment

bruno-garcia Dec 1, 2023

Choose a reason for hiding this comment

matejminar Dec 1, 2023

Choose a reason for hiding this comment

romtsn Dec 1, 2023

Choose a reason for hiding this comment

bruno-garcia Dec 1, 2023

Choose a reason for hiding this comment

bruno-garcia Dec 1, 2023

Choose a reason for hiding this comment

bruno-garcia Dec 1, 2023

Choose a reason for hiding this comment

kanadaj commented Dec 1, 2023

bruno-garcia left a comment

Choose a reason for hiding this comment

bruno-garcia commented Dec 1, 2023

kanadaj commented Dec 1, 2023

bitsandfoxes commented Dec 1, 2023 • edited Loading

bruno-garcia commented Dec 1, 2023

kanadaj commented Dec 1, 2023 • edited Loading

bitsandfoxes commented Dec 1, 2023 •

edited

Loading

kanadaj commented Dec 1, 2023 •

edited

Loading