Add support for Sentry Kotlin Compiler Plugin by markushi · Pull Request #2695 · getsentry/sentry-java

markushi · 2023-05-04T11:33:42Z

📜 Description

Counterpart for Kotlin Compiler Plugin

💡 Motivation and Context

Adds compose support for automatic user interactions tracking
Add View Hierarchy support

💚 How did you test it?

📝 Checklist

I reviewed the submitted code.
I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Counterpart for Kotlin Compiler Plugin

…on tracking

github-actions · 2023-05-04T11:34:44Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 57d1a71

github-actions · 2023-05-04T11:53:42Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	317.14 ms	342.80 ms	25.66 ms
Size	1.72 MiB	2.28 MiB	570.71 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`46b1782`	387.72 ms	458.74 ms	71.02 ms
`1707044`	338.80 ms	384.79 ms	46.00 ms

App size

Revision	Plain	With Sentry	Diff
`46b1782`	1.72 MiB	2.28 MiB	570.44 KiB
`1707044`	1.72 MiB	2.28 MiB	570.44 KiB

Previous results on branch: feat/compose-compiler-plugin

Startup times

Revision	Plain	With Sentry	Diff
`5ea803d`	326.18 ms	359.36 ms	33.18 ms
`ffa0ad4`	289.42 ms	376.22 ms	86.80 ms
`ea7fcbe`	367.34 ms	414.72 ms	47.38 ms
`775fa63`	298.58 ms	388.15 ms	89.56 ms

App size

Revision	Plain	With Sentry	Diff
`5ea803d`	1.72 MiB	2.28 MiB	570.71 KiB
`ffa0ad4`	1.72 MiB	2.28 MiB	565.75 KiB
`ea7fcbe`	1.72 MiB	2.26 MiB	553.74 KiB
`775fa63`	1.72 MiB	2.26 MiB	553.74 KiB

codecov · 2023-05-04T11:57:15Z

Codecov Report

Patch coverage: 20.00% and project coverage change: -0.02 ⚠️

Comparison is base (a485ab0) 81.11% compared to head (826f763) 81.09%.

❗ Current head 826f763 differs from pull request most recent head 57d1a71. Consider uploading reports for the commit 57d1a71 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2695      +/-   ##
============================================
- Coverage     81.11%   81.09%   -0.02%     
  Complexity     4447     4447              
============================================
  Files           345      345              
  Lines         16394    16399       +5     
  Branches       2226     2226              
============================================
+ Hits          13298    13299       +1     
- Misses         2168     2172       +4     
  Partials        928      928

Impacted Files	Coverage Δ
sentry/src/main/java/io/sentry/SentryOptions.java	`79.95% <20.00%> (-0.63%)`	⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

romtsn

Code-wise this looks good, but I have few rather conceptual things:

When I tried integrating kotlin-compiler-plugin into the sample, I've faced a compilation error, because our sample uses Kotlin 1.8, and the compiler plugin seems to support only 1.8.20 onwards (after updating the kotlin version, it worked). The error was the same one as drewhamilton/Poko#122. The question is if we want to support lower version of Kotlin, or we just say we support the latest one onwards?
I thought of moving SentryModifier and SentryComposeTracing to commonMain if possible, but I guess we could do it later when we start looking into compose-multiplatform support
Last thing - I've tried integrating the compiler plugin into our sample, but unfortunately I don't think it brings much value in the current state (unless I misunderstood how it all should work in the end). E.g. for our sample on the screenshot I've removed all of the custom SentryTraced wrappers and just left the compiler plugin do its job and it only inferred one tag (Landing). I guess we should rethink our instrumentation a bit for the view hierarchy case 😅

markushi · 2023-05-23T10:50:17Z

Code-wise this looks good, but I have few rather conceptual things:

When I tried integrating kotlin-compiler-plugin into the sample, I've faced a compilation error, because our sample uses Kotlin 1.8, and the compiler plugin seems to support only 1.8.20 onwards (after updating the kotlin version, it worked). The error was the same one as Kotlin 1.8.20 support drewhamilton/Poko#122. The question is if we want to support lower version of Kotlin, or we just say we support the latest one onwards?

I thought of moving SentryModifier and SentryComposeTracing to commonMain if possible, but I guess we could do it later when we start looking into compose-multiplatform support

Last thing - I've tried integrating the compiler plugin into our sample, but unfortunately I don't think it brings much value in the current state (unless I misunderstood how it all should work in the end). E.g. for our sample on the screenshot I've removed all of the custom SentryTraced wrappers and just left the compiler plugin do its job and it only inferred one tag (Landing). I guess we should rethink our instrumentation a bit for the view hierarchy case 😅

Yes, this is a known issue. The only alternative would be to release different versions of our kotlin compiler plugin and have a compatibility matrix, just like Jetpack Compose has, but as of now this would also mean having different versions of our AGP as well. I'd stick to the 1.8.20 requirement.
Sounds good!
This looks odd, there should definitely be more metadata in there. Let me have a closer look at this.

romtsn · 2023-05-23T14:07:33Z

Yes, this is a known issue. The only alternative would be to release different versions of our kotlin compiler plugin and have a compatibility matrix, just like Jetpack Compose has, but as of now this would also mean having different versions of our AGP as well. I'd stick to the 1.8.20 requirement.

I guess we'll still have to maintain the compat matrix for the future versions of Kotlin, but yeah let's start with 1.8.20 as the initial version

romtsn · 2023-05-25T11:38:33Z


+### Features
+- Add support for Sentry Kotlin Compiler Plugin ([#2695](https://github.com/getsentry/sentry-java/pull/2695))
+  - In conjunction with our sentry-kotlin-compiler-plugin we improved Jetpack Compose support for


nit: Ideally here would be also a link to sentry-kotlin-compiler-plugin docs, but we can update it later ;)

I'd really appreciate a link to the compiler plugin docs as I've been meaning to try these new features and haven't been able to understand how to use the compiler plugin.

@tfcporciuncula they are in the making, but soon will be there :) getsentry/sentry-docs#7026

markushi added 3 commits April 25, 2023 07:18

Add SentryModifier

c32471a

Counterpart for Kotlin Compiler Plugin

Match spec with Kotlin Compiler plugin, integrate into user interacti…

0878998

…on tracking

Add support for exporting JPC View Hierarchies

808125e

markushi requested review from adinauer, romtsn and stefanosiano as code owners May 4, 2023 11:33

Update Changelog

8ff091b

markushi changed the title ~~Add support for Sentry Compose Compiler Plugin~~ Add support for Sentry Kotlin Compiler Plugin May 4, 2023

markushi added 4 commits May 4, 2023 20:29

Exclude ComposeViewHierarchyExporter from obfuscation

343a3dd

Merge branch 'main' into feat/compose-compiler-plugin

166dbc6

Integrate Compose View Hierarchy Exporter into sentry-android-core

d81b7f3

Fix changelog

727e964

romtsn reviewed May 19, 2023

View reviewed changes

Comment thread sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java Outdated

romtsn reviewed May 20, 2023

View reviewed changes

Comment thread CHANGELOG.md Outdated

romtsn reviewed May 22, 2023

View reviewed changes

Comment thread ...se-helper/src/jvmMain/java/io/sentry/compose/viewhierarchy/ComposeViewHierarchyExporter.java Outdated

romtsn reviewed May 22, 2023

View reviewed changes

Comment thread ...elper/src/jvmTest/java/io/sentry/compose/viewhierarchy/ComposeViewHierarchyExporterTest.java

romtsn approved these changes May 22, 2023

View reviewed changes

markushi added 3 commits May 25, 2023 07:46

Merge branch 'main' into feat/compose-compiler-plugin

5c89890

Exclude layoutDelegate from minification

826f763

Lazy init and re-use SentryComposeHelper

57d1a71

romtsn reviewed May 25, 2023

View reviewed changes

markushi merged commit 46e0307 into main May 26, 2023

markushi deleted the feat/compose-compiler-plugin branch May 26, 2023 05:16

Uh oh!

Uh oh!

Conversation

markushi commented May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

github-actions Bot commented May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/compose-compiler-plugin

Startup times

App size

Uh oh!

codecov Bot commented May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romtsn left a comment

Choose a reason for hiding this comment

Uh oh!

markushi commented May 23, 2023

Uh oh!

romtsn commented May 23, 2023

Uh oh!

romtsn May 25, 2023

Choose a reason for hiding this comment

Uh oh!

tfcporciuncula Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

romtsn Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

markushi commented May 4, 2023 •

edited

Loading

github-actions Bot commented May 4, 2023 •

edited

Loading

github-actions Bot commented May 4, 2023 •

edited

Loading

codecov Bot commented May 4, 2023 •

edited

Loading