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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic performance instrumentation for WebFlux #2597

Merged
merged 9 commits into from
Mar 14, 2023

Conversation

adinauer
Copy link
Member

📜 Description

This adds automatic performance instrumentation for WebFlux

💡 Motivation and Context

Closes #1807

💚 How did you test it?

Manually using samples + automated tests

📝 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 Mar 10, 2023

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

Generated by 🚫 dangerJS against 4a1cd90

@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 361.43 ms 409.41 ms 47.98 ms
Size 1.73 MiB 2.34 MiB 626.68 KiB

Previous results on branch: feat/webflux-performance

Startup times

Revision Plain With Sentry Diff
8ee0aeb 299.94 ms 349.60 ms 49.66 ms
a18a30a 339.39 ms 382.51 ms 43.12 ms
f78fb8d 421.55 ms 458.24 ms 36.69 ms
89d7ccf 343.42 ms 434.04 ms 90.62 ms
776fb7f 335.74 ms 361.26 ms 25.52 ms

App size

Revision Plain With Sentry Diff
8ee0aeb 1.73 MiB 2.34 MiB 626.25 KiB
a18a30a 1.73 MiB 2.34 MiB 626.56 KiB
f78fb8d 1.73 MiB 2.34 MiB 626.25 KiB
89d7ccf 1.73 MiB 2.34 MiB 626.46 KiB
776fb7f 1.73 MiB 2.34 MiB 626.46 KiB

@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Patch coverage: 80.58% and project coverage change: -0.01 ⚠️

Comparison is base (674b462) 81.16% compared to head (4a1cd90) 81.16%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2597      +/-   ##
============================================
- Coverage     81.16%   81.16%   -0.01%     
- Complexity     4125     4149      +24     
============================================
  Files           333      335       +2     
  Lines         15329    15432     +103     
  Branches       1995     2011      +16     
============================================
+ Hits          12442    12525      +83     
- Misses         2101     2111      +10     
- Partials        786      796      +10     
Impacted Files Coverage Δ
...g/boot/jakarta/SentryWebfluxAutoConfiguration.java 80.00% <0.00%> (-5.72%) ⬇️
...ry/spring/boot/SentryWebfluxAutoConfiguration.java 57.14% <0.00%> (-9.53%) ⬇️
...spring/jakarta/webflux/SentryWebTracingFilter.java 82.00% <82.00%> (ø)
.../sentry/spring/webflux/SentryWebTracingFilter.java 82.00% <82.00%> (ø)
...java/io/sentry/spring/webflux/SentryWebFilter.java 96.15% <100.00%> (+0.15%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@adinauer adinauer merged commit 46f5dfd into main Mar 14, 2023
@adinauer adinauer deleted the feat/webflux-performance branch March 14, 2023 05:45
@adinauer adinauer mentioned this pull request Mar 22, 2023
4 tasks
@yhware
Copy link
Contributor

yhware commented May 10, 2023

Hi @adinauer
Since this feature was added, I'm not sure which one I should use for tracing.

  1. The one implemented here
  2. Opentelemetry integration https://docs.sentry.io/platforms/java/performance/instrumentation/opentelemetry/

Do you guys have a recommendation?

@adinauer
Copy link
Member Author

@yhware if you're already using OpenTelemetry, you'll probably want to use our OpenTelemetry Java Agent. Otherwise feel free to give this here a try first. If you have any feedback, feel free to share 🙏 .

@yhware
Copy link
Contributor

yhware commented May 12, 2023

Somehow the opentelemetry option is taking up a few hundred megs of memory and causing our dev containers to get OOMKilled.
But removing the opentelemetry option and updating sdk to the latest version does not give us performance metrics.
Is there a required spring/jdk version for it to work?

I enabled sentry.enable-tracing: true

@adinauer
Copy link
Member Author

@yhware have you set sample rate to something < 1.0 or instrumenter to otel in your options? Otherwise try to enable debug, maybe that tells you something.

@adinauer
Copy link
Member Author

We'll have to investigate memory consumption of the OTEL integration. Can you please share more details on what you did, like is it just simple HTTP request or something special? How long does it take to run OOM? What version of Spring (Boot) are you on?

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.

Automatic performance transactions for Webflux
3 participants