Replace version in definition schema with file_format#1154
Replace version in definition schema with file_format#1154lmolkova wants to merge 4 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1154 +/- ##
=====================================
Coverage 80.1% 80.1%
=====================================
Files 108 108
Lines 8440 8440
=====================================
+ Hits 6761 6762 +1
+ Misses 1679 1678 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -1,4 +1,4 @@ | |||
| version: "2" | |||
| file_format: definition/2.0.0 | |||
There was a problem hiding this comment.
Nit: I'm not sure we want a three-version rule on language syntax. Or at least we should answer some questions:
- When do we fail to load a file because of major/minor versions? E.g. do we need something in place where weaver will fail to resolve
definition/2.1.0right now because we haven't defined the syntax there? - When will we bump this version? Should we do so on any major feature addition to the syntax? (probably). We should open a ticket to have some kind of automated tracker to let us know to do this so it doesn't get lost.
- Do we need a tool to migrate from version 2.x -> 2.y? I had started working on such a thing for 1->2.
I'm on board moving this direction of using file_format everywhere, and we can make these decisions over time, but I'd like to write down our versioning/loading policy a bit more (it can be in follow on bugs) and make sure our release that uses this syntax can "scale" to future versions/usage.
| V1(SemConvSpecV1), | ||
| /// Version 2 of the semantic convention schema. | ||
| #[serde(rename = "2")] | ||
| #[serde(rename = "definition/2.0.0")] |
There was a problem hiding this comment.
This will ONLY handle the literal string definition/2.0.0. If we were to try a version bump, like definition/2.1.0 then this would fail to resolve previous versions.
I think there's some HARD implications here for us to sort through. E.g. I'd prefer If we had a custom deserialization path for SERDE that would resolve version string and then delegate to a deserializer of choice.
That custom path would be responsible for
- knowing if the version that came is compatible with versions we can represent in the current weaver.
- Choosing the right Rust structure to deserialize into.
Align definition schema marker with resolved schema introduced in #1136
Extracted from #1106