Refactor ReactorUtils into its own sentry-reactor module#4155
Conversation
|
The commented out tests in Edit: I was not initializing the SDK :) Also the ThreadLocalAccessor can be set up with SPI instead. |
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| c2c78de | 415.28 ms | 505.08 ms | 89.80 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| c2c78de | 1.58 MiB | 2.21 MiB | 640.27 KiB |
Previous results on branch: lcian/ref/reactor-as-module
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7276711 | 444.02 ms | 475.80 ms | 31.78 ms |
| 67c204c | 397.52 ms | 481.22 ms | 83.70 ms |
| c320936 | 369.81 ms | 481.23 ms | 111.42 ms |
| 74b0da5 | 380.19 ms | 454.58 ms | 74.40 ms |
| 4bbe412 | 423.04 ms | 466.40 ms | 43.36 ms |
| 10216d9 | 482.39 ms | 514.26 ms | 31.87 ms |
| 3dd4220 | 373.60 ms | 468.00 ms | 94.40 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7276711 | 1.58 MiB | 2.21 MiB | 640.27 KiB |
| 67c204c | 1.58 MiB | 2.21 MiB | 640.25 KiB |
| c320936 | 1.58 MiB | 2.21 MiB | 640.27 KiB |
| 74b0da5 | 1.58 MiB | 2.21 MiB | 640.27 KiB |
| 4bbe412 | 1.58 MiB | 2.21 MiB | 640.27 KiB |
| 10216d9 | 1.58 MiB | 2.21 MiB | 640.25 KiB |
| 3dd4220 | 1.58 MiB | 2.21 MiB | 640.27 KiB |
sentry-reactor/src/main/java/io/sentry/reactor/SentryReactorThreadLocalAccessor.java
Show resolved
Hide resolved
|
Other things that might be missing from this PR:
We need manually add this new module to Docs would also be great. |
adinauer
left a comment
There was a problem hiding this comment.
LGTM!
We shouldn't forget to update release registry, then update .craft.yml here.
Also would be great to have an entry in the root README listing this new module.
|
Thanks @adinauer . I'll add this to the README after it's released, otherwise we'll have broken links. I'll add this to the issue for v8.3.0 so we don't forget about the missing tasks. |
📜 Description
Refactor
ReactorUtilsinto its ownsentry-reactormodule💡 Motivation and Context
Part of #3144
💚 How did you test it?
Unit tests and manual tests with the
webfluxsamples.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Pending actions:
Comments
No, it will still be included in the Spring and Spring Boot integrations, just as a different module.
We need to add an entry to the
sentry-release-registryand then update.craft.yml(post-release)We will add a deprecation warning and make the old classes extend the new ones, so we don't break existing customers.