Conversation
Inspect the Spring Environment before SDK initialization and emit tailored migration warnings for explicit sentry.metrics.enabled values without binding or applying the obsolete property. Co-Authored-By: Claude <noreply@anthropic.com>
|
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
…y-metrics-spring # Conflicts: # sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryAutoConfiguration.java # sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java # sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java
| .getLogger() | ||
| .log( | ||
| SentryLevel.WARNING, | ||
| "The 'sentry.metrics.enabled' property is no longer supported. Manual " |
There was a problem hiding this comment.
That string is duplicated all over the place, should we create a constant for it?
| .getLogger() | ||
| .log( | ||
| SentryLevel.WARNING, | ||
| "The 'sentry.metrics.enabled' property no longer disables manual " |
There was a problem hiding this comment.
That string is duplicated all over the place, should we create a constant for it?
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: An invalid value for the sentry.metrics.enabled property will cause an unhandled ConversionFailedException during startup, crashing the application.
Severity: HIGH
Suggested Fix
Wrap the environment.getProperty("sentry.metrics.enabled", Boolean.class) call in a try-catch block to handle potential ConversionFailedException. If an invalid value is detected, log a warning and proceed with a sensible default value to avoid crashing the application on startup.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryAutoConfiguration.java#L242
Potential issue: The code calls `environment.getProperty("sentry.metrics.enabled",
Boolean.class)` without error handling. If a user configures this property with a string
that Spring's `ConversionService` cannot interpret as a boolean (e.g., "enabled" or
"active"), a `ConversionFailedException` will be thrown. Because this property is read
during the creation of the `sentryHub` bean at application startup, the unhandled
exception will propagate and cause the entire application to fail to start.
Also affects:
sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java:244~244sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java:239~239
Did we get this right? 👍 / 👎 to inform future reviews.
PR Stack (Logs and Metrics Enable Flags)
📜 Description
Detects explicit
sentry.metrics.enabledconfiguration through Spring'sEnvironmentin all three Spring Boot variants. Migration warnings are emitted afterSentry.init, when the diagnostic logger has been initialized.Both
trueandfalsevalues emit tailored migration warnings. The obsolete property is not added back toSentryPropertiesand does not affect manual Metrics capture. Absent configuration emits no warning.💡 Motivation and Context
After removal of the aggregate Metrics option, Spring's relaxed binder ignores the old property. Inspecting the complete Spring
Environmentpreserves a useful migration diagnostic across property files, YAML, environment variables, command-line values, and other property sources.💚 How did you test it?
./gradlew :sentry-spring-boot:test :sentry-spring-boot-jakarta:test :sentry-spring-boot-4:test./gradlew spotlessApply apiDumptrue, and explicitfalsetests to all three Spring Boot variants📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Avoid starting the Metrics batch worker thread until the first accepted Metrics item.
#skip-changelog