Fix java.lang.ClassNotFoundException: org.springframework.web.servlet.HandlerMapping in Spring Boot Servlet mode without WebMVC #3336

adinauer · 2024-04-09T06:07:54Z

📜 Description

HandlerMapping is used by SpringMvcTransactionNameProvider. We now provide a fallback TransactionNameProvider which uses the requestURI as transaction name. This means we're not using the route which contains placeholders for IDs (e.g. /todo/{id}) but instead use e.g. /todo/100.

💡 Motivation and Context

Fix-es #1863 (2nd part)

💚 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

github-actions · 2024-04-09T06:16:50Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	424.08 ms	475.80 ms	51.71 ms
Size	1.70 MiB	2.28 MiB	592.12 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`99a51e2`	405.11 ms	479.65 ms	74.54 ms
`baaf637`	375.71 ms	441.58 ms	65.87 ms
`95a98b5`	379.14 ms	420.80 ms	41.66 ms
`8ff8fd0`	432.77 ms	495.18 ms	62.41 ms
`937879e`	400.98 ms	482.84 ms	81.86 ms
`4e29063`	376.38 ms	390.98 ms	14.60 ms
`28c9a83`	366.20 ms	433.88 ms	67.67 ms
`0bd723b`	412.52 ms	496.65 ms	84.13 ms
`93a76ca`	377.96 ms	447.52 ms	69.56 ms
`a537f8a`	457.14 ms	514.19 ms	57.05 ms

App size

Revision	Plain	With Sentry	Diff
`99a51e2`	1.72 MiB	2.29 MiB	576.34 KiB
`baaf637`	1.72 MiB	2.27 MiB	558.42 KiB
`95a98b5`	1.70 MiB	2.27 MiB	586.31 KiB
`8ff8fd0`	1.72 MiB	2.27 MiB	558.15 KiB
`937879e`	1.72 MiB	2.27 MiB	558.42 KiB
`4e29063`	1.72 MiB	2.29 MiB	578.38 KiB
`28c9a83`	1.70 MiB	2.28 MiB	592.00 KiB
`0bd723b`	1.72 MiB	2.29 MiB	578.09 KiB
`93a76ca`	1.72 MiB	2.29 MiB	576.75 KiB
`a537f8a`	1.70 MiB	2.27 MiB	584.74 KiB

Previous results on branch: fix/missing-handler-mapping-class

Startup times

Revision	Plain	With Sentry	Diff
`e52c71c`	372.09 ms	445.53 ms	73.44 ms
`d9a27b7`	388.57 ms	456.52 ms	67.95 ms

App size

Revision	Plain	With Sentry	Diff
`e52c71c`	1.70 MiB	2.28 MiB	592.05 KiB
`d9a27b7`	1.70 MiB	2.28 MiB	592.05 KiB

lbloder

Code changes look good 👍

Tests need to be adapted, as the ones testing the missing HandlerExceptionResolver fail due to the TransactionNameProvider missing now too.

lbloder · 2024-04-09T06:38:15Z

...spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java

@@ -300,7 +294,7 @@ public FilterRegistrationBean<SentryTracingFilter> sentryTracingFilter(
      @Configuration(proxyBeanMethods = false)
      @ConditionalOnClass(HandlerExceptionResolver.class)
      @Open
-      static class SentryExceptionResolverConfigurationWrapper {
+      static class SentryNonServletOnlyModeConfig {


Just a suggestion, but wouldn't something like SentryMvcModeConfig, be more easily understandable?

yeah probably

lbloder · 2024-04-09T06:38:22Z

sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java

@@ -298,7 +292,7 @@ public FilterRegistrationBean<SentryTracingFilter> sentryTracingFilter(
      @Configuration(proxyBeanMethods = false)
      @ConditionalOnClass(HandlerExceptionResolver.class)
      @Open
-      static class SentryExceptionResolverConfigurationWrapper {
+      static class SentryNonServletOnlyModeConfig {


Just a suggestion, but wouldn't something like SentryMvcModeConfig, be more easily understandable?

…ns URL

lbloder

LGTM 👍
Added suggestion to add an additional test

lbloder · 2024-04-09T09:22:01Z

...ng-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt

@@ -445,6 +447,15 @@ class SentryAutoConfigurationTest {
            }
    }

+    @Test
+    fun `when Spring MVC is not on the classpath, fallback TransactionNameProvider is configured`() {


(l) would it make sense to also add a test to assert that, if MVC is used, the SpringMvcTransactionNameProvider is instantiated?

Yeah we can add this kind of test later to not block this PR.

lbloder · 2024-04-09T09:22:12Z

sentry-spring-boot/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt

@@ -444,6 +446,17 @@ class SentryAutoConfigurationTest {
            }
    }

+    @Test
+    fun `when Spring MVC is not on the classpath, fallback TransactionNameProvider is configured`() {


(l) would it make sense to also add a test to assert that, if MVC is used, the SpringMvcTransactionNameProvider is instantiated?

Yeah we can add this kind of test later to not block this PR.

Fix missing HandlerMapping class

851f1ee

adinauer requested a review from lbloder April 9, 2024 06:07

adinauer requested review from romtsn, stefanosiano and markushi as code owners April 9, 2024 06:07

changelog

b50e285

lbloder reviewed Apr 9, 2024

View reviewed changes

adinauer and others added 3 commits April 9, 2024 09:09

rename config

87c6fed

provide default transaction name provider for servlet mode that retur…

d2de6ee

…ns URL

Merge branch 'main' into fix/missing-handler-mapping-class

aeb3496

lbloder approved these changes Apr 9, 2024

View reviewed changes

adinauer merged commit 39e8942 into main Apr 9, 2024
24 checks passed

adinauer deleted the fix/missing-handler-mapping-class branch April 9, 2024 10:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix java.lang.ClassNotFoundException: org.springframework.web.servlet.HandlerMapping in Spring Boot Servlet mode without WebMVC #3336

Fix java.lang.ClassNotFoundException: org.springframework.web.servlet.HandlerMapping in Spring Boot Servlet mode without WebMVC #3336

adinauer commented Apr 9, 2024 •

edited

Loading

github-actions bot commented Apr 9, 2024 •

edited

Loading

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/missing-handler-mapping-class

Startup times

App size

lbloder left a comment

lbloder Apr 9, 2024

adinauer Apr 9, 2024

lbloder Apr 9, 2024

lbloder left a comment

lbloder Apr 9, 2024

adinauer Apr 9, 2024

lbloder Apr 9, 2024

adinauer Apr 9, 2024

Fix java.lang.ClassNotFoundException: org.springframework.web.servlet.HandlerMapping in Spring Boot Servlet mode without WebMVC #3336

Fix java.lang.ClassNotFoundException: org.springframework.web.servlet.HandlerMapping in Spring Boot Servlet mode without WebMVC #3336

Conversation

adinauer commented Apr 9, 2024 • edited Loading

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Apr 9, 2024 • edited Loading

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/missing-handler-mapping-class

Startup times

App size

lbloder left a comment

Choose a reason for hiding this comment

lbloder Apr 9, 2024

Choose a reason for hiding this comment

adinauer Apr 9, 2024

Choose a reason for hiding this comment

lbloder Apr 9, 2024

Choose a reason for hiding this comment

lbloder left a comment

Choose a reason for hiding this comment

lbloder Apr 9, 2024

Choose a reason for hiding this comment

adinauer Apr 9, 2024

Choose a reason for hiding this comment

lbloder Apr 9, 2024

Choose a reason for hiding this comment

adinauer Apr 9, 2024

Choose a reason for hiding this comment

adinauer commented Apr 9, 2024 •

edited

Loading

github-actions bot commented Apr 9, 2024 •

edited

Loading