Allow cross-registry refinements#1587
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes cross-registry (dependency) refinement/extends resolution by preserving the extended group’s signal type during resolution, enabling v1→v2 conversion and inheritance to work when the parent group is not present in the local registry.
Changes:
- Record the parent group’s
GroupTypeinGroupLineageduringextendsresolution. - Improve v2 dependency group lookup to handle both prefixed (v1-style) and unprefixed (v2-style) IDs and to preserve per-attribute requirement levels/sampling relevance.
- Add end-to-end resolver fixtures/tests covering metric/span/event (and an ignored entity) refinements over a published v2 dependency.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/weaver_resolver/src/registry.rs | Stores extended group type in lineage during extends resolution and continues inheriting required v2 fields. |
| crates/weaver_resolver/src/lib.rs | Adds end-to-end tests asserting resolved v2 output for cross-dependency refinements. |
| crates/weaver_resolver/src/dependency.rs | Improves v2 dependency lookup (prefix-stripping) and attribute inheritance fidelity (requirement level, sampling relevance). |
| crates/weaver_resolver/data/registry-test-v2-dep/span_registry/registry/registry.yaml | New v2 span refinement fixture referencing a dependency span. |
| crates/weaver_resolver/data/registry-test-v2-dep/span_registry/registry/manifest.yaml | New fixture manifest declaring dependency. |
| crates/weaver_resolver/data/registry-test-v2-dep/span_registry/expected-schema.yaml | Expected resolved v2 output for span refinement fixture. |
| crates/weaver_resolver/data/registry-test-v2-dep/published/resolved.yaml | Expands published dependency fixture to include span/event/entity signals and attributes. |
| crates/weaver_resolver/data/registry-test-v2-dep/metric_registry/registry/registry.yaml | New v2 metric refinement fixture referencing a dependency metric. |
| crates/weaver_resolver/data/registry-test-v2-dep/metric_registry/registry/manifest.yaml | New fixture manifest declaring dependency. |
| crates/weaver_resolver/data/registry-test-v2-dep/metric_registry/expected-schema.yaml | Expected resolved v2 output for metric refinement fixture. |
| crates/weaver_resolver/data/registry-test-v2-dep/event_registry/registry/registry.yaml | New v2 event refinement fixture referencing a dependency event. |
| crates/weaver_resolver/data/registry-test-v2-dep/event_registry/registry/manifest.yaml | New fixture manifest declaring dependency. |
| crates/weaver_resolver/data/registry-test-v2-dep/event_registry/expected-schema.yaml | Expected resolved v2 output for event refinement fixture. |
| crates/weaver_resolver/data/registry-test-v2-dep/entity_registry/registry/registry.yaml | New v2 entity refinement fixture (currently exercised by an ignored test). |
| crates/weaver_resolver/data/registry-test-v2-dep/entity_registry/registry/manifest.yaml | New fixture manifest declaring dependency. |
| crates/weaver_resolved_schema/src/v2/mod.rs | Switches refinement detection to rely on the stored extended-group type during v1→v2 conversion. |
| crates/weaver_resolved_schema/src/lineage.rs | Adds non-serialized extends_group_type to GroupLineage and updates extends() signature. |
| CHANGELOG.md | Notes new support for refinements over published dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome. For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question. Automation flags a PR for human review once every review thread has a reply or is marked as resolved. Status across open PRs is visible on the pull request dashboard. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1587 +/- ##
=====================================
Coverage 82.3% 82.3%
=====================================
Files 129 129
Lines 11000 10941 -59
=====================================
- Hits 9061 9013 -48
+ Misses 1939 1928 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jsuereth
left a comment
There was a problem hiding this comment.
Nice cleanup / extension!
| ); | ||
| if let Some(lineage) = unresolved_group.group.lineage.as_mut() { | ||
| lineage.extends(extends); | ||
| lineage.extends(extends, parent_summary.r#type.clone()); |
There was a problem hiding this comment.
I find it amusing this is the key fix here :)
When refining something from dependency, weaver fails with
fixing it