Skip to content

Tolerate unknown properties in resolved schema / published manifest#1365

Open
lmolkova wants to merge 6 commits intoopen-telemetry:mainfrom
lmolkova:tolerate-future
Open

Tolerate unknown properties in resolved schema / published manifest#1365
lmolkova wants to merge 6 commits intoopen-telemetry:mainfrom
lmolkova:tolerate-future

Conversation

@lmolkova
Copy link
Copy Markdown
Member

@lmolkova lmolkova commented Apr 16, 2026

Older weaver can now read files written by a newer minor version.

  • Newer minor (resolved/2.X on a 2.0 build): loads, unknowns ignored, warning that newer version is available is logged.
  • Major mismatch (e.g. resolved/3.0): not supported, fatal error at start
  • Same/older minor with unknown field: fatal error listing unexpected fields (path). e.g. registry.spans[0].unknown_field

Warning

It introduces regression for definition side (we reuse quite a few types between definition and resolved).
Since deny_unknown_fields is relaxed, we'll tolerate errors in definitions that we failed on before.

Definitions PR: #1422

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 95.86777% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.4%. Comparing base (37c645a) to head (ade7409).

Files with missing lines Patch % Lines
crates/weaver_semconv/src/manifest.rs 69.2% 4 Missing ⚠️
crates/weaver_resolver/src/loader.rs 85.7% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1365     +/-   ##
=======================================
+ Coverage   82.2%   82.4%   +0.2%     
=======================================
  Files        125     127      +2     
  Lines      10225   10325    +100     
=======================================
+ Hits        8405    8509    +104     
+ Misses      1820    1816      -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Apr 27, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@lmolkova lmolkova marked this pull request as ready for review April 28, 2026 02:04
@lmolkova lmolkova requested a review from a team as a code owner April 28, 2026 02:04
///
/// Serializes and deserializes as the string form (e.g. `"resolved/2.0"`).
#[derive(Clone, PartialEq, Eq, Hash)]
pub struct FileFormat {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmolkova I approve of most of this PR - but wanted to check in on your plans based on the discussion today.

I'm fine leaving this here with some flexibility we discussed, or addressing file-format expansion in a later PR.

Should I approve this PR as-is?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR does not touch anything related to definition, I was going to address it in separate PR (within this release). So I think this one can go as is

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's the definition part #1422

Comment thread crates/weaver_resolver/src/loader.rs Fixed
Comment thread crates/weaver_resolver/src/loader.rs Fixed
Comment thread crates/weaver_resolver/src/loader.rs Fixed
Comment thread crates/weaver_resolver/src/loader.rs Fixed
@lmolkova lmolkova force-pushed the tolerate-future branch from c3b109c to e50c3f5 Compare May 1, 2026 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants