feat: logcat integration#455
Conversation
|
markushi
left a comment
markushi
left a comment
There was a problem hiding this comment.
This is great, well done! 🚀 please have a look at my comments
There was a problem hiding this comment.
LGTM, besides a couple of things.
We should probably also add an integration test that verifies the instrumented bytecode can be accepted by the JVM. You could look at VisitorTest and just add a new .class fixture with some logcat calls in it and then test it analogous to other fixtures in that test.
|
Nicely done! You can definitely mark the PR as "read for review" now.
|
|
@buenaflor is this going to work for dependencies and transitive dependencies? |
Yes, bytecode manipulation transforms all bytecode (app + all bundled dependencies) |
|
|
||
| ## Unreleased | ||
|
|
||
| ### Features |
There was a problem hiding this comment.
btw the changelog is outdated, you'd need to merge against main again
4 participants
📜 Description
Adds a
logcatsubextension that has following options:enabledminLevelminLeveldetermines which Log functions will be replaced bySentryLogcatAdaptervia bytecode manipulation.SentryLogcatAdapterwill add breadcrumbs to the respective log.This depends on: getsentry/sentry-java#2620
💡 Motivation and Context
Ref: getsentry/sentry-java#1620
💚 How did you test it?
📝 Checklist
🔮 Next steps
Update sentry-docs