-
Notifications
You must be signed in to change notification settings - Fork 941
fix(config): Add version mismatch warning for declarative config #8069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(config): Add version mismatch warning for declarative config #8069
Conversation
Logs warning when file_format doesn't exactly match expected version (1.0.0-rc.3). - Add EXPECTED_FILE_FORMAT constant - Update regex to support 1.0.0-rc.N format - Add tests for warning behavior
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8069 +/- ##
=========================================
Coverage 90.20% 90.20%
- Complexity 7592 7595 +3
=========================================
Files 841 841
Lines 22911 22919 +8
Branches 2288 2291 +3
=========================================
+ Hits 20666 20674 +8
Misses 1529 1529
Partials 716 716 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| Logger.getLogger(OpenTelemetryConfigurationFactory.class.getName()); | ||
| private static final Pattern SUPPORTED_FILE_FORMATS = | ||
| Pattern.compile("^(0.4)|(1.0(-rc.\\d*)?)|(1.0.0-rc.\\d*)$"); | ||
| private static final String EXPECTED_FILE_FORMAT = "1.0.0-rc.3"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file format is supposed to omit the patch version. See docs here: https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema-docs.md#opentelemetryconfiguration-
The idea is that since patches will only be for bug fixes, there should never be something actionable on the part of an implementation, and so forcing or even allowing the user to specify the patch version adds possible confusion by implying that it matters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jack-berg - I've updated the regex to not use the patch number in 2289467
Patch versions = bug fixes only, no user action needed.
jack-berg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes a TODO left by @jack-berg in the configuration file to check for valid schema version and log a warning when it does not match.