Conversation
Changelog Preview📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧Deps
🤖 This preview updates automatically when you update the PR. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| # better performance, it's recommended to keep it enabled if possible. | ||
| # | ||
| # The default value is `true`. | ||
| dns_cache: false |
There was a problem hiding this comment.
Bug: The dns_cache: false setting in config.example.yml is not commented out, which will disable DNS caching by default for all new installations, degrading performance.
Severity: MEDIUM
Suggested Fix
Comment out the dns_cache: false line and its parent http key in relay/config.example.yml. This will make it an optional setting that users can enable if needed, consistent with other workarounds in the file.
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: relay/config.example.yml#L34
Potential issue: The `config.example.yml` file, used for new installations, now includes
`dns_cache: false` as an active, uncommented setting. This contradicts the file's own
comments, which describe it as a workaround for specific DNS issues and state the
default is `true`. Other workarounds in the file are commented out by default. By
leaving this setting active, all new self-hosted installations will experience
performance degradation from disabled DNS caching, even if they do not have the
underlying DNS problem this setting is intended to solve.
Did we get this right? 👍 / 👎 to inform future reviews.
aminvakil
left a comment
aminvakil
left a comment
There was a problem hiding this comment.
I do not think this is a good default to disable all dns queries caching.
aminvakil
left a comment
aminvakil
left a comment
There was a problem hiding this comment.
Honestly I'm still on both sides about merging this as it is, or with dns_cache: true by default.
@aminvakil yeah lets hold this off for a while |
2 participants
Resolves getsentry/sentry#109002