Rework Resolution Engine to allow using resolved schema by jsuereth · Pull Request #1136 · open-telemetry/weaver

jsuereth · 2026-01-13T13:43:33Z

Reworks the resolution engine to allow using resolved schema as a dependency.

Breaks phases formally into Loaded and Resolved
Loaded can now work with a loaded ResolvedTelemetrySchema (v1 or v2) or a definition schema (raw / unresolved).
Resolve now takes loaded, and will recurse down dependencies as needed.

TODOS

Remove legacy resolve code from code base
Handle uniqueness of group / signal ids across dependencies
More tests

…esolving a new schema.

crates/weaver_semconv/src/registry_repo.rs

codecov · 2026-01-13T13:53:24Z

Codecov Report

❌ Patch coverage is 77.27273% with 110 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.1%. Comparing base (bf4c7a9) to head (aace775).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/weaver_semconv/src/lib.rs	28.0%	41 Missing ⚠️
crates/weaver_resolver/src/loader.rs	79.5%	30 Missing ⚠️
crates/weaver_resolver/src/error.rs	38.0%	13 Missing ⚠️
crates/weaver_resolver/src/dependency.rs	87.0%	12 Missing ⚠️
crates/weaver_resolver/src/lib.rs	83.7%	7 Missing ⚠️
crates/weaver_resolver/src/attribute.rs	92.5%	3 Missing ⚠️
crates/weaver_resolved_schema/src/lib.rs	50.0%	2 Missing ⚠️
crates/weaver_common/src/result.rs	85.7%	1 Missing ⚠️
crates/weaver_resolver/src/registry.rs	98.1%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1136     +/-   ##
=======================================
- Coverage   80.2%   80.1%   -0.2%     
=======================================
  Files        105     108      +3     
  Lines       8126    8428    +302     
=======================================
+ Hits        6523    6753    +230     
- Misses      1603    1675     +72

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

crates/weaver_semconv/src/lib.rs

…resolution.

…rences.

crates/weaver_resolver/src/loader.rs

crates/weaver_resolver/src/registry.rs

crates/weaver_resolver/src/loader.rs

crates/weaver_resolver/src/registry.rs

crates/weaver_resolver/src/error.rs

…lved-schema

crates/weaver_forge/src/v2/registry.rs

crates/weaver_resolved_schema/src/v2/mod.rs

crates/weaver_resolver/src/attribute.rs

crates/weaver_semconv/src/manifest.rs

crates/weaver_semconv/src/stats.rs

crates/weaver_resolver/src/registry.rs

crates/weaver_resolved_schema/src/v2/mod.rs

crates/weaver_resolver/src/loader.rs

Co-authored-by: Liudmila Molkova <neskazu@gmail.com>

lquerel · 2026-01-21T06:41:38Z

crates/weaver_resolver/src/attribute.rs

+            .map(|(a, ar)| (a.clone(), *ar))
+            .collect();
+        ordered.sort_by(|(ln, _), (rn, _)| ln.cmp(rn));
+        // assert_eq!(ordered.len(), self.attribute_refs.len());


The kind of thing that could use debug_assert_eq!.

Good call! I missed this before merging, I'll remember it going forward.

lquerel · 2026-01-21T06:53:46Z

crates/weaver_resolver/src/attribute.rs

+
+impl AttributeLookup for V1Schema {
+    fn lookup_attribute(&self, key: &str) -> Option<AttributeWithGroupId> {
+        // TODO - fast lookup, not a table scan...


I thought we originally had a map. Why did we lose that? It;s possible my memory is failing me.

Ordering. You switched to a linear order for serialization.

I'll need to lead a map on the side.

lquerel

I only noticed a few fairly minor issues (the main one is potentially around the imports).
I know that in this PR you didn't try to optimize lookups in the registry/catalog, with find_map used in several places. I think that, in the long run, we'll need to address this, as it could become problematic, especially with support for multiple registries.

lquerel · 2026-01-21T07:10:11Z

crates/weaver_resolver/src/lib.rs

+        let registry_id: String = repo.id().to_string();
+        let manifest = repo.manifest().cloned();
+        let mut attr_catalog = AttributeCatalog::default();
+        // TODO - Do something with non_fatal_errors if we need to.


Why not merge the non fatal errors coming from the dependencies with the WResult returned by this method? It feels like, at the moment, we're simply dropping those errors.

We are, there's a bug on this being the preferred default.

I can write in the flag that will allow ignoring/dropping or preserving, but I'm going to default to ignoring.

The issue at least before, was using current rules and rego policies on older repositories. That may be less of an issue pulling resolved registry but it's still not as useful as you'd think

lquerel · 2026-01-21T07:22:05Z

crates/weaver_resolver/src/dependency.rs

+        let metrics_imports_matcher = build_globset(
+            current_registry_imports
+                .iter()
+                .find_map(|i| i.imports.metrics.as_ref()),


Can't we have imports across multiple files? With find_map here, we’re only finding the first one.

Yeah. This is a temporary quick implementation while we only support one. This method will need to be fleshed out to support multiple later, but at least it's contained in one spot now.

Hum. Maybe I misunderstood what you were trying to do here. It doesn't feel like this is about supporting multiple dependencies, but rather about the imports section in a semconv file, which allows defining a subset of metrics to import, the wildcard mechanism. Am I mistaken, or does using find_map prevent capturing all imports?

lquerel · 2026-01-21T07:39:24Z

crates/weaver_resolver/src/loader.rs

+                            dependency_chain,
+                        ) {
+                            WResult::Ok(d) => loaded_dependencies.push(d),
+                            // TODO - Should we always ignore warnings on loaded dependencies?


I don't think we should ignore non-fatal errors on loaded dependencies. We don't really have any guaranties that they are well-formed.

Updated this to previous behavior and have TODO around #1126.

PTAL at that issue and comment your thoughts. I'll defer fixing it.

jsuereth added 14 commits January 9, 2026 09:42

use consistent file versions for resolved schema.

ecc9f54

More cleanups in resolved schema.

b31194f

Initial scaffolding for new resolution algorithm.

fbe1d7b

Start to move manifest to match Semconv Schema 2.0.0 format.

803577a

Add missing fields to manifest.

6facc0a

More scaffolding of new algorithm.

29ff153

Basic resolution algorithm.

393b2d9

Scaffold out mechanism to be able to use resolved dependencies when r…

ae15c3e

…esolving a new schema.

Get dependency key lookup working for V1 schema.

2ee898e

Add notes for where to fix next.

f4aa844

Add scaffolding for importing dependencies from resolved schemas.

9c2d95e

Get tests to pass, even if we have a huge attirbute reference issue.

2e6b1df

Get all weaver resolver multi-registry tests updated and passing.

6795003

Begin removing old resolution methods from codebase.

207e21f

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

View reviewed changes

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

jsuereth added 2 commits January 13, 2026 11:08

Clippy fixes.

18c7c97

Add attribute ordering so we can have deterministic resolved schemas.

2dc13b5

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

View reviewed changes

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

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

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

jsuereth added 9 commits January 14, 2026 11:35

Fix sorting and add tests around sorting and resulting registry from …

8840569

…resolution.

Fix erroneous / bad bug around which groups were used for GC'ing refe…

fd71a05

…rences.

Some test fixes.

d28ed0b

cargo fmt

2e9c1d8

Various restructuring and fixes.

d2cc3d4

All resolver tests pass.

5b1df0d

Fix remaining build issues.

d476978

More cleanups and fixes.

9e4bfe0

Clippy.

c1e1581

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

View reviewed changes

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

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

Update forge expected output for new sorting.

7b63f5f

jsuereth changed the title ~~[DRAFT] Rework Resolution Engine to allow using resolved schema~~ Rework Resolution Engine to allow using resolved schema Jan 15, 2026

Add more test coverage for dependencies.

e9881da

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

View reviewed changes

jsuereth added 2 commits January 15, 2026 15:51

Add basic test for YAML value sorting.

d02b66f

Fmt and clippy fixes.

257f116

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

View reviewed changes

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

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

Fix typo.

69387aa

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

View reviewed changes

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

jsuereth added 3 commits January 16, 2026 09:47

Clippy fixes.

b1cef6d

Improve test.

55c08f6

Merge remote-tracking branch 'origin/main' into wip-resolve-from-reso…

0a7a5fb

…lved-schema

lmolkova reviewed Jan 16, 2026

View reviewed changes

jsuereth and others added 4 commits January 16, 2026 17:57

Update crates/weaver_forge/src/v2/registry.rs

101c5ae

Co-authored-by: Liudmila Molkova <neskazu@gmail.com>

Update crates/weaver_semconv/src/manifest.rs

09c9764

Co-authored-by: Liudmila Molkova <neskazu@gmail.com>

Update crates/weaver_resolver/src/registry.rs

b5bd50b

Co-authored-by: Liudmila Molkova <neskazu@gmail.com>

Cargo fmt

d6c113c

lmolkova approved these changes Jan 17, 2026

View reviewed changes

lmolkova mentioned this pull request Jan 17, 2026

First draft of the section "Building and publishing arbitrary semantic convention registries" lmolkova/opentelemetry-specification#3

Closed

jsuereth added 3 commits January 17, 2026 12:42

Fixes from review.

995e1f5

Fix broken test from example update

bb53721

cargo fmt

4e156cc

lquerel reviewed Jan 21, 2026

View reviewed changes

lquerel approved these changes Jan 21, 2026

View reviewed changes

Fixes from review.

aace775

jsuereth enabled auto-merge (squash) January 21, 2026 13:39

jsuereth merged commit 556e05f into open-telemetry:main Jan 21, 2026
21 checks passed

lmolkova mentioned this pull request Jan 23, 2026

Replace version in definition schema with file_format #1154

Merged

jsuereth deleted the wip-resolve-from-resolved-schema branch January 26, 2026 23:23

michaelvanstraten mentioned this pull request Mar 19, 2026

Allow suppression of warnings for dependencies. #1126

Open

Conversation

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

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 13, 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

lquerel left a comment

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jsuereth commented Jan 13, 2026 •

edited

Loading

codecov bot commented Jan 13, 2026 •

edited

Loading