Allow resolving from resolved schema using Manifest file. by jsuereth · Pull Request #1160 · open-telemetry/weaver

jsuereth · 2026-01-26T20:24:08Z

Fixes #1134

Should allow specifying a schema like in OTEP 4815.

Can specify a manifest instead of a directory, e.g. http://my-schema.org/manifest/1.0.0
Will use the resolved_schema_url, if available, on that manifest to resolve schema instead of grabbing .yaml files from the directory.
Allow relative URLs for resolved_schema_url, although this needs to be tested a bit.

Thing not working yet:

Extending groups from V2
Actually using manifest.yaml vs. registry_manifest.yaml as per OTEP 4815.

…han just information on dependencies

…emas.

…DirectoryRef

crates/weaver_common/src/vdir.rs

codecov · 2026-01-26T20:34:17Z

Codecov Report

❌ Patch coverage is 84.46602% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.2%. Comparing base (8c1dfdb) to head (637eecb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/weaver_common/src/vdir.rs	33.3%	12 Missing ⚠️
crates/weaver_resolver/src/loader.rs	70.0%	9 Missing ⚠️
crates/weaver_resolver/src/dependency.rs	93.9%	7 Missing ⚠️
crates/weaver_common/src/lib.rs	75.0%	2 Missing ⚠️
crates/weaver_resolver/src/attribute.rs	94.7%	1 Missing ⚠️
crates/weaver_semconv/src/registry_repo.rs	93.3%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1160     +/-   ##
=======================================
+ Coverage   79.9%   80.2%   +0.3%     
=======================================
  Files        109     109             
  Lines       8586    8769    +183     
=======================================
+ Hits        6865    7040    +175     
- Misses      1721    1729      +8

☔ 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.

…st multi-dependency test.

crates/weaver_semconv/src/registry_repo.rs

crates/weaver_resolver/src/dependency.rs

crates/weaver_resolver/src/loader.rs

jerbly

This is a big one... I'm going to try this branch out. In the meantime, I found a couple of bugs and some nits.

crates/weaver_resolver/src/dependency.rs

crates/weaver_common/src/vdir.rs

crates/weaver_resolver/src/loader.rs

crates/weaver_resolver/src/dependency.rs

jerbly · 2026-02-08T00:33:42Z

crates/weaver_semconv/src/registry_repo.rs

+                Some(self.registry.vdir_path().map_sub_folder(|path| {
+                    if vdir_was_manifest_file {
+                        match Path::new(&path).parent() {
+                            Some(parent) => format!("{}/{resolved_url}", parent.display()),


Is this right? parent.display() will use os dependent separators, so backslashes in Windows.

It will only do that if it's parsed or you use join. I wrote this on windows, and yes it's a bit awkward. Probably the thing that needs the MOST attention in this PR.

If you want me to refactor this, I'd be happy to try a bit.

crates/weaver_resolver/src/error.rs

lmolkova · 2026-02-09T21:39:26Z

crates/weaver_semconv/src/manifest.rs

+    /// This can be either:
+    /// - A manifest of a published registry
+    /// - A directory containing the raw definition.
    pub registry_path: VirtualDirectoryPath,


can we call it accordingly? could be confusing to see registry_path: http://otel.io/schemas ...

Suggested change

pub registry_path: VirtualDirectoryPath,

pub registry_location: VirtualDirectoryPath,

or

Suggested change

pub registry_path: VirtualDirectoryPath,

pub registry: VirtualDirectoryPath,

Since manifest work today and folks use them, I didn't want to issue breaking changes.

I'm happy to move to something else and declare an alias here so we don't break.

Personally, I prefer registry_uri or registry_location better. The key thing to denote is this is a "virtual directory path" in Weaver vernacular, so it can point at directories inside a git repository or zip archive.

Should I update that now?

now is great, I can also do it in #1106 (registry_uri sounds good and we'd align registy_url with it)

Since this is a rather large PR - I'd rather do a dedicate PR for that change.

I think we could also look at moving from registry_manifest.yaml -> manifest.yaml in that one.

lmolkova · 2026-02-09T21:42:36Z

crates/weaver_resolver/src/error.rs

+    #[error("Maximum dependency depth reached for registry `{registry}`. Cannot load further dependencies.")]
+    MaximumDependencyDepth {
+        /// The registry which has too many dependencies.
+        registry: String,


can we align on registry vs registry_id (above) vs registry_url ?

Sure - I'm fine with either. Which do you think makes a better error message? URL or ID?

I was playing with it in the #1106 and I'm thinking:

registry_url | uri is required and necessary

id is now equivalent to name, it's not essential - can be optional on dependency

what if we had name() method that'd return name if it's known or fell back to url if not. We'd use it when reporting dependency errors and then called it registry_name (since that's what it called on dependency and in the manifest).

OR we should rename dependency.name and manifest.name to id and then use registry_id everywhere

So - we need a "name" that's separate from "version" so we can isolate if we have a repeated/diamond dependency. That's actually orthogonal to this discussion, but made me realize we may have a bug in our dependency tracking on that front.

I'm a fan of name() method that falls back from human readable to the url. I need to sort through the whole codebase for id and fix it.

Do you think we can do that in a separate PR or do you want to clean that up now?

yeah, let's do it as a follow-up - that'd be easier to review

Yes! Let's do that in a follow up PR - This gets enough done I can start fragmenting my work into separate branches again without everything conflicting.

jerbly

Approving this as it's intended as a first in a series of PRs. In a subsequent PR it would be good to focus on usability around error reporting for mistakes made in manifest files. Is there a schema for the manifest file? And, we should tackle the todo!()s and .expect()s to avoid panics.

jsuereth · 2026-02-11T12:10:01Z

Approving this as it's intended as a first in a series of PRs. In a subsequent PR it would be good to focus on usability around error reporting for mistakes made in manifest files. Is there a schema for the manifest file? And, we should tackle the todo!()s and .expect()s to avoid panics.

Agreed there is more cleanup to do here.

@jerbly - Is there a todo or expect that's in non test code you're worried about? I'm aware of one around recursive dependencies that needs to be tackled, but I don't remember any expect() calls outside of tests. If you saw one, that should be fixed ASAP - I don't see one myself though.

crates/weaver_resolver/src/dependency.rs

jerbly · 2026-02-11T13:13:20Z

Is there a todo or expect that's in non test code you're worried about? I'm aware of one around recursive dependencies that needs to be tackled, but I don't remember any expect() calls outside of tests. If you saw one, that should be fixed ASAP - I don't see one myself though.

I commented them for you.... You have TODOs for them. These should be proper errors e.g. DanglingAttributeReference??. Likewise a MultipleDependenciesNotSupported would be good.

jsuereth · 2026-02-11T13:17:36Z

Thanks @jerbly - I'll clean this up before submitting

crates/weaver_resolver/src/dependency.rs

…ifest

jsuereth and others added 3 commits January 26, 2026 08:43

Start making changes to use the manifest during resolution for more t…

cb800c4

…han just information on dependencies

Update capabilities of RegistryRepo to allow referencing resolved sch…

cb32b9d

…emas.

Clean up registry repo manifest reading to make better use of Virtual…

b55ac71

…DirectoryRef

github-advanced-security bot found potential problems Jan 26, 2026

View reviewed changes

crates/weaver_common/src/vdir.rs Fixed Show fixed Hide fixed

crates/weaver_common/src/vdir.rs Fixed Show fixed Hide fixed

crates/weaver_common/src/vdir.rs Fixed Show fixed Hide fixed

jsuereth added 7 commits January 26, 2026 16:01

Clean up errors and loading to allow loading V2 resolved schema.

34ee08a

Fix diagnostic severity not flowing through transparent errors.

ff6b470

Refactor registry resolve tests for multi-dependencies, and start fir…

613f6af

…st multi-dependency test.

Get simple metric import working from V2 registry.

7b35508

Complete an import implementation.

77ff50b

Add attribute lookup test.

3ca9298

Fix clippy.

ef27d97

jsuereth marked this pull request as ready for review January 28, 2026 14:13

jsuereth requested a review from a team as a code owner January 28, 2026 14:13

github-advanced-security bot found potential problems Jan 28, 2026

View reviewed changes

crates/weaver_semconv/src/registry_repo.rs Fixed Show fixed Hide fixed

jsuereth changed the title ~~[DRAFT] Allow resolving from resolved schema using Manifest file.~~ Allow resolving from resolved schema using Manifest file. Jan 28, 2026

Fix windows path handling

946891c

github-advanced-security bot found potential problems Jan 30, 2026

View reviewed changes

crates/weaver_semconv/src/registry_repo.rs Fixed Show fixed Hide fixed

Fix typos and clippy.

88a63a9

github-advanced-security bot found potential problems Jan 30, 2026

View reviewed changes

jsuereth added 2 commits January 30, 2026 12:03

Clippy fixes

8367685

Dead code cleanup

a18e5a6

jerbly reviewed Feb 8, 2026

View reviewed changes

Fix from review.

899eb01

lmolkova reviewed Feb 9, 2026

View reviewed changes

Fix from review.

9cea92a

lmolkova mentioned this pull request Feb 9, 2026

Add multi-registry documentation with schema v2 format #1190

Closed

30 tasks

lmolkova approved these changes Feb 10, 2026

View reviewed changes

jsuereth mentioned this pull request Feb 10, 2026

Update naming conventions for multi-registry manifests #1197

Closed

jerbly approved these changes Feb 11, 2026

View reviewed changes

jerbly reviewed Feb 11, 2026

View reviewed changes

crates/weaver_resolver/src/dependency.rs Outdated Show resolved Hide resolved

jerbly reviewed Feb 11, 2026

View reviewed changes

crates/weaver_resolver/src/dependency.rs Outdated Show resolved Hide resolved

jerbly reviewed Feb 11, 2026

View reviewed changes

crates/weaver_resolver/src/dependency.rs Outdated Show resolved Hide resolved

jerbly reviewed Feb 11, 2026

View reviewed changes

crates/weaver_resolver/src/dependency.rs Outdated Show resolved Hide resolved

Remove expects for real errors.

99af1c5

github-advanced-security bot found potential problems Feb 11, 2026

View reviewed changes

crates/weaver_resolver/src/dependency.rs Fixed Show fixed Hide fixed

crates/weaver_resolver/src/dependency.rs Fixed Show fixed Hide fixed

crates/weaver_resolver/src/dependency.rs Fixed Show fixed Hide fixed

crates/weaver_resolver/src/dependency.rs Fixed Show fixed Hide fixed

jsuereth added 3 commits February 11, 2026 11:08

Merge branch 'main' into wip-resolve-using-manifest

de7fb63

Clippy.

530faea

Merge remote-tracking branch 'origin/main' into wip-resolve-using-man…

637eecb

…ifest

jsuereth enabled auto-merge (squash) February 12, 2026 21:11

jsuereth merged commit 7020e15 into open-telemetry:main Feb 12, 2026
22 checks passed

	pub registry_path: VirtualDirectoryPath,
	pub registry_location: VirtualDirectoryPath,

	pub registry_path: VirtualDirectoryPath,
	pub registry: VirtualDirectoryPath,

Conversation

jsuereth commented Jan 26, 2026 • edited by lmolkova Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jerbly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerbly left a comment

Choose a reason for hiding this comment

Uh oh!

jsuereth commented Feb 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jerbly commented Feb 11, 2026

Uh oh!

jsuereth commented Feb 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jsuereth commented Jan 26, 2026 •

edited by lmolkova

Loading

codecov bot commented Jan 26, 2026 •

edited

Loading