feat(spring): [Logs and Metrics Enable Flags 10] Warn for legacy Logs property - #5951
Open
adinauer wants to merge 3 commits into
Open
feat(spring): [Logs and Metrics Enable Flags 10] Warn for legacy Logs property#5951adinauer wants to merge 3 commits into
adinauer wants to merge 3 commits into
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
Inspect the Spring Environment before SDK initialization and emit tailored migration warnings for explicit sentry.logs.enabled values without binding or applying the obsolete property. Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor
|
Sorry, something went wrong.
This was referenced Aug 13, 2026
Copy link
Copy Markdown
📲 Install BuildsAndroid
|
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Performance metrics 🚀
Baseline results on branch: feat/warn-legacy-logs-externalStartup times
App size
Previous results on branch: feat/warn-legacy-logs-springStartup times
App size
|
Sorry, something went wrong.
This was referenced Aug 13, 2026
This was referenced Aug 13, 2026
Emit legacy Logs property diagnostics after Sentry.init so the configured diagnostic logger is available. Co-Authored-By: Claude <noreply@anthropic.com>
8 tasks
adinauer
marked this pull request as ready for review
August 13, 2026 13:41
adinauer
requested review from
markushi,
romtsn and
runningcode
as code owners
August 13, 2026 13:41
Comment thread
...spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java
Comment on lines
+200
to
+201
| Boolean.TRUE.equals(environment.getProperty("sentry.logs.enabled", Boolean.class)); | ||
| if (enableLogs) { |
Copy link
Copy Markdown
There was a problem hiding this comment.
Bug: The application will crash on startup if sentry.logs.enabled is set to a non-boolean value, as the call to environment.getProperty lacks error handling for invalid conversions.
Severity: HIGH
Wrap the environment.getProperty("sentry.logs.enabled", Boolean.class) call in a try-catch block to handle ConversionFailedException. Log a warning if the property value is invalid and default to a sensible value (e.g., false) to allow the application to start successfully. This makes the configuration more robust against user error.
Prompt for AI AgentReview 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-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java#L200-L201 Potential issue: The code checks for the existence of the `sentry.logs.enabled` property but does not validate its value before attempting to convert it to a `Boolean`. If a user provides a non-boolean string (e.g., "enabled" instead of "true"), Spring's `StringToBooleanConverter` will throw a `ConversionFailedException`. Since this exception is not caught during the creation of the `sentryHub` bean, it will propagate up and cause the entire Spring application context to fail to initialize, preventing the application from starting.
Also affects:
- sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java:195~196
- sentry-spring-boot-4/src/main/java/io/sentry/spring/boot4/SentryAutoConfiguration.java:198~199
Did we get this right? 👍 / 👎 to inform future reviews.
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
if this is not a regression then this is nothing to worry about. we probably want to fail here anyways.
Sorry, something went wrong.
runningcode
approved these changes
Aug 14, 2026
This was referenced Aug 14, 2026
lbloder
approved these changes
Aug 23, 2026
lbloder
left a comment
Copy link
Copy Markdown
Collaborator
There was a problem hiding this comment.
LGTM 👍
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Stack (Logs and Metrics Enable Flags)
📜 Description
Injects the Spring Environment into SDK initialization for all three Spring Boot variants and detects explicit sentry.logs.enabled configuration. Migration warnings are emitted after Sentry.init, when the diagnostic logger has been initialized.
Both true and false values emit tailored migration warnings. The obsolete property is not restored to SentryProperties and does not affect capture or the new sentry.logging.logs-enabled integration control. Absent configuration emits no warning.
💡 Motivation and Context
After removal of the aggregate Logs option, Spring's relaxed binder ignores the old property. Inspecting the complete Spring Environment preserves a useful migration diagnostic across property files, YAML, environment variables, command-line values, and other property sources.
💚 How did you test it?
📝 Checklist
🔮 Next steps
Avoid starting the Logs batch worker thread until the first accepted Log item.
#skip-changelog