feat: Send SDK Integration List, add missing packages by romtsn · Pull Request #2179 · getsentry/sentry-java

romtsn · 2022-07-20T08:29:31Z

📜 Description

Changed List to Set because we don't care about duplicates (and also, it's easier to use, e.g. for Navigation listener, which can be attached/detached multiple times during the app lifetime)
It's also a synchronized Set since it can be modified from different threads
Keep List getter as to not introduce breaking changes
An easier would be to change the signature of register method which would return true/false depending on whether the integration is enabled or not, then we could add all of them on the registring site. But this would mean breaking the public API so I opted out from that
Adapt/add tests
Introduced SentryIntegrationPackageStorage Singleton to correctly handle Java integrations like log4J, logback and OTEL that might initialize Sentry but get overriden by e.g. Spring Boot.
Introduced IntegrationName interface

outdated

You can see the event with integrations here https://sentry.io/organizations/sentry-sdks/issues/3402041535/events/e085890223004873b274c7bb6c6d1c8f/json/

💡 Motivation and Context

Closes #1885

🔮 Next steps

After #initial implementation to write enabled instrumentations to file sentry-android-gradle-plugin#421 lands, read integrations introduced by the Sentry Android Gradle Plugin from manifest and add them to the list

github-actions · 2022-07-20T08:30:11Z

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

Generated by 🚫 dangerJS against 594cc5e

# Conflicts: # sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java # sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java # sentry/src/main/java/io/sentry/UncaughtExceptionHandlerIntegration.java

github-actions · 2022-12-15T11:06:47Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	324.45 ms	380.57 ms	56.12 ms
Size	1.73 MiB	2.34 MiB	626.30 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`4b32504`	315.69 ms	373.96 ms	58.27 ms
`754021c`	358.70 ms	361.98 ms	3.28 ms
`fe30606`	310.82 ms	335.36 ms	24.55 ms
`14c083a`	350.82 ms	388.86 ms	38.04 ms
`4b32504`	357.14 ms	404.04 ms	46.90 ms
`c1399c1`	345.06 ms	385.49 ms	40.43 ms
`f6a135d`	263.96 ms	383.59 ms	119.63 ms
`5fa24ec`	326.29 ms	384.53 ms	58.24 ms
`efa7b3d`	330.06 ms	370.14 ms	40.08 ms
`efa7b3d`	335.38 ms	384.85 ms	49.48 ms

App size

Revision	Plain	With Sentry	Diff
`4b32504`	1.73 MiB	2.34 MiB	623.74 KiB
`754021c`	1.73 MiB	2.33 MiB	623.06 KiB
`fe30606`	1.73 MiB	2.34 MiB	623.74 KiB
`14c083a`	1.73 MiB	2.33 MiB	620.61 KiB
`4b32504`	1.73 MiB	2.34 MiB	623.74 KiB
`c1399c1`	1.73 MiB	2.33 MiB	620.61 KiB
`f6a135d`	1.73 MiB	2.33 MiB	623.10 KiB
`5fa24ec`	1.73 MiB	2.33 MiB	620.61 KiB
`efa7b3d`	1.73 MiB	2.34 MiB	625.56 KiB
`efa7b3d`	1.73 MiB	2.34 MiB	625.56 KiB

Previous results on branch: feat/integrations-in-events

Startup times

Revision	Plain	With Sentry	Diff
`607bde5`	312.84 ms	341.02 ms	28.18 ms
`f2ac750`	331.21 ms	382.12 ms	50.92 ms
`47b5673`	317.63 ms	363.37 ms	45.74 ms
`13749c3`	335.81 ms	367.06 ms	31.25 ms
`3556d6d`	273.42 ms	318.63 ms	45.22 ms
`20115be`	339.14 ms	381.59 ms	42.44 ms
`9668485`	347.31 ms	370.49 ms	23.18 ms
`8789ba4`	355.11 ms	394.53 ms	39.42 ms
`6b688da`	374.96 ms	422.16 ms	47.20 ms
`d1e381e`	300.51 ms	369.57 ms	69.06 ms

App size

Revision	Plain	With Sentry	Diff
`607bde5`	1.73 MiB	2.33 MiB	621.17 KiB
`f2ac750`	1.73 MiB	2.34 MiB	626.06 KiB
`47b5673`	1.73 MiB	2.34 MiB	626.08 KiB
`13749c3`	1.73 MiB	2.33 MiB	621.17 KiB
`3556d6d`	1.73 MiB	2.33 MiB	614.83 KiB
`20115be`	1.73 MiB	2.34 MiB	626.01 KiB
`9668485`	1.73 MiB	2.34 MiB	626.03 KiB
`8789ba4`	1.73 MiB	2.34 MiB	626.08 KiB
`6b688da`	1.73 MiB	2.34 MiB	626.08 KiB
`d1e381e`	1.73 MiB	2.34 MiB	624.19 KiB

…ntry-java into feat/integrations-in-events

# Conflicts: # sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java

romtsn · 2022-12-29T09:12:25Z

@lbloder what do you think of Philipp's suggestion? At least for those that implement the Integration interface, we could get the class name and remove the Integration part from its name when we call register. This would reduce the maintenance overhead for those that implement Integration at least.

romtsn · 2022-12-29T09:23:12Z

@lbloder can I also ask you to add the missing integrations to packages (sdkVersion.addPackage())? This would also help us to analyze data based on packages. The modules that need this to be added:

okhttp
apollo2
apollo3
fragment
navigation
compose
compose-helper (also needs to be added to the integrations list)

Don't know about jdbc and graphql, since you haven't added them to the integrations yet

# Conflicts: # sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java

…ntry-java into feat/integrations-in-events

adinauer

We need to find a different way of handling deserialization. Clearing all packages / integrations would lead to bad data.

adinauer · 2023-02-16T08:34:23Z


 - Add time-to-full-display span to Activity auto-instrumentation ([#2432](https://github.com/getsentry/sentry-java/pull/2432))
 - Add `main` flag to threads and `in_foreground` flag for app contexts  ([#2516](https://github.com/getsentry/sentry-java/pull/2516))
+- Add integrations to integration list ([#2179](https://github.com/getsentry/sentry-java/pull/2179))


Maybe put this under Fixes and call it:

Integrations and packages in use are now reported more consistently

wdyt?

romtsn · 2023-02-16T11:14:04Z

 public final class ComposeGestureTargetLocator implements GestureTargetLocator {

+  public ComposeGestureTargetLocator() {
+    SentryIntegrationPackageStorage.getInstance().addIntegration("ComposeHelper");


Suggested change

SentryIntegrationPackageStorage.getInstance().addIntegration("ComposeHelper");

SentryIntegrationPackageStorage.getInstance().addIntegration("ComposeUserInteraction");

SentryIntegrationPackageStorage.getInstance().addPackage("maven:io.sentry:sentry-compose", BuildConfig.VERSION_NAME)

since it's embedded in sentry-compose

romtsn · 2023-02-16T11:16:35Z

One more ask - could you please add SentryIntegrationPackageStorage.getInstance().addIntegration("FileIO"); to the FileIOSpanManager ctor? this way we don't need to wait for gradle plugin to inject its features, but can already track FileIO usages. Thanks 🙏

…ge for integrationLIst getter

… getter add getter for set

…res/ComposeGestureTargetLocator.java Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>

adinauer

Just finished this off by fixing compile, adding BuildConfig to compose-helper and adding a test to ensure storage doesn't affect deserialized SdkVersion instances.

WIP initial implementation for sending SDK integrations list

de92b48

Format code

6f0e125

philippsuess reviewed Aug 29, 2022

View reviewed changes

lbloder and others added 3 commits December 14, 2022 16:00

add java integrations to integration list

3d261c1

Format code

0ed5f76

lbloder added 5 commits December 16, 2022 09:16

new additions to integration list

629e7b9

Merge branch 'feat/integrations-in-events' of github.com:getsentry/se…

54c8ad9

…ntry-java into feat/integrations-in-events

Merge branch 'main' into feat/integrations-in-events

663a02c

# Conflicts: # sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java

Merge branch 'main' into feat/integrations-in-events

bcec4e7

read enabled android gradle plugin integrations from file

c99e41b