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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

add property to disable AOP related features that break graalvm build #2915

Merged
merged 8 commits into from Sep 18, 2023

Conversation

lbloder
Copy link
Collaborator

@lbloder lbloder commented Sep 5, 2023

馃摐 Description

Add property sentry.enable-aot-compatibility to disable AOP, which breaks GraalVM Builds.

馃挕 Motivation and Context

Currently it's not possible to build for GraalVM with performance enabled due to AOP on @SentryTransaction and @SentrySpan. With this change performance can be enabled, but the AOP functionality is turned off.

Fixes #2742

馃挌 How did you test it?

Add id("org.graalvm.buildtools.native") version "0.9.24" plugin to build.gradle and run the bootBuildImage command.

馃摑 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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

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

Generated by 馃毇 dangerJS against 6b5b3f6

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Performance metrics 馃殌

Plain With Sentry Diff
Startup time 325.52 ms 363.72 ms 38.20 ms
Size 1.72 MiB 2.29 MiB 575.88 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
4bf202b 364.28 ms 419.66 ms 55.38 ms
9246ed4 281.79 ms 352.08 ms 70.29 ms
3baa73f 267.45 ms 388.30 ms 120.85 ms
496bdfd 272.86 ms 407.33 ms 134.48 ms
9e60fc1 310.00 ms 364.42 ms 54.42 ms
549cbb4 332.89 ms 384.44 ms 51.55 ms
8820c5c 330.60 ms 416.86 ms 86.26 ms
4bf202b 331.20 ms 345.24 ms 14.04 ms
fb296f0 340.61 ms 394.88 ms 54.27 ms
0f34a0c 321.75 ms 377.68 ms 55.93 ms

App size

Revision Plain With Sentry Diff
4bf202b 1.72 MiB 2.29 MiB 575.54 KiB
9246ed4 1.72 MiB 2.28 MiB 572.22 KiB
3baa73f 1.72 MiB 2.29 MiB 575.52 KiB
496bdfd 1.72 MiB 2.28 MiB 571.82 KiB
9e60fc1 1.72 MiB 2.29 MiB 575.91 KiB
549cbb4 1.72 MiB 2.29 MiB 575.74 KiB
8820c5c 1.72 MiB 2.28 MiB 571.82 KiB
4bf202b 1.72 MiB 2.29 MiB 575.54 KiB
fb296f0 1.72 MiB 2.29 MiB 575.70 KiB
0f34a0c 1.72 MiB 2.29 MiB 575.74 KiB

Previous results on branch: feat/property_disable_aop

Startup times

Revision Plain With Sentry Diff
1cf9e84 327.80 ms 371.94 ms 44.14 ms
4e473d2 310.57 ms 362.45 ms 51.88 ms
cd111b7 370.80 ms 423.86 ms 53.06 ms
770bd4a 309.08 ms 352.71 ms 43.63 ms
bb068c1 333.53 ms 381.47 ms 47.94 ms

App size

Revision Plain With Sentry Diff
1cf9e84 1.72 MiB 2.29 MiB 575.94 KiB
4e473d2 1.72 MiB 2.29 MiB 575.91 KiB
cd111b7 1.72 MiB 2.29 MiB 575.90 KiB
770bd4a 1.72 MiB 2.29 MiB 575.94 KiB
bb068c1 1.72 MiB 2.29 MiB 575.94 KiB

@codecov
Copy link

codecov bot commented Sep 5, 2023

Codecov Report

Patch coverage is 25.00% of modified lines.

Files Changed Coverage
...y/spring/boot/jakarta/SentryAutoConfiguration.java
...o/sentry/spring/boot/jakarta/SentryProperties.java 25.00%

馃摙 Thoughts on this report? Let us know!.

Copy link
Member

@adinauer adinauer left a comment

Choose a reason for hiding this comment

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

LGTM, just lacks a comment in SentryProperties to explain the flag.

@lbloder lbloder merged commit 6c21863 into main Sep 18, 2023
19 of 21 checks passed
@lbloder lbloder deleted the feat/property_disable_aop branch September 18, 2023 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DSN is required. Use empty string to disable SDK when building Native in Spring Boot
2 participants