Initial implementatoin of V2 lineage.#1277
Initial implementatoin of V2 lineage.#1277jsuereth wants to merge 3 commits intoopen-telemetry:mainfrom
Conversation
lmolkova
left a comment
There was a problem hiding this comment.
I'm a bit lost on why lineage is important and why we need it on resolved schema :)
| ], | ||
| "brief": "A test attribute group", | ||
| "stability": "stable", | ||
| "lineage": { |
There was a problem hiding this comment.
do we want lineage in resolve schema? it's going to really bloat it and it does not seem to be essential for telemetry consumers. TBH I struggle to understand why is needed in any cases but debugging. I think it makes sense to provide it in the forge one.
Even better: it seems to belong in the debug log when resolving a registry.
The dependency resolution is a separate story, but it would not need that much.
There was a problem hiding this comment.
I think this is a crucial dependency for us handling multi-imports in the future. For V1 - lineage was crucial for making sure we could transition to v2 and we would not be where we are today without it.
So I do disagree here. I understand the concern, and it's possible we need to go with a far more lightweight lineage tracking design here, but as i was working on allowing multiple "parent" registries to be defined, lineage was necessary to resolve ambiguities and import-issues across registires.
| brief: 'A test span refinement.' | ||
| note: 'This note overrides the base span imported from source.' | ||
| attributes: | ||
| - ref: custom.attr2 |
There was a problem hiding this comment.
why does it work? there are no dependencies in the manifest
| "attributes": { | ||
| "custom.attr2": { | ||
| "source": { | ||
| "raw_attribute": "span.test.base" |
There was a problem hiding this comment.
span.test.base is not even included in this resolved schema file (why?), but also the most important information is which registry this attr came from and there is no indication of it.
| "attribute_groups": [], | ||
| "spans": [ | ||
| { | ||
| "type": "test.base.app.refinement", |
There was a problem hiding this comment.
why refinement is in the span type?
| }, | ||
| "custom.attr2": { | ||
| "source": { | ||
| "attribute_group": "registry.data.registry-test-lineage-v2-1-base-signals.registry.base" |
There was a problem hiding this comment.
shouldn't it be raw_attribute ? it seems it's coming through v1 and becomes irrelevant to the test case (definition v2)
| "attributes": { | ||
| "custom.attr2": { | ||
| "source": { | ||
| "raw_attribute": "span.test.base" |
There was a problem hiding this comment.
why raw_attribute ? I don't quite understand what it means. If it's
/// The attribute is defined as a free-floating attribute not tied to any group.
then why does it have span.test.base as the source?
| "path": "data/registry-test-lineage-v2-2-refinements/registry/refinements.yaml" | ||
| }, | ||
| "attributes": { | ||
| "custom.attr1": { |
There was a problem hiding this comment.
why it's an attribute key and not an index in attr catalog?
also why attrs in catalog have no lineage - they are definitions and refinements and if we need lineage, it should be on the attr in the catalog and then there will be less duplication on signals
There was a problem hiding this comment.
Right - I did the most naive implementation for now - This is kind of what I wanted to investigate.
I'm still considering the actual lineage tracking design, and the important pieces of it.
For the most part, I need to know when a signal or attribute (or attirbute group) came from a specific schema, so if we have multiple imports I can resolve that these are actually the same, and deal with version conflicts appropriately. This is still draft so we can collectively consider that use case and what we need.
An initial implementation of lineage for V2.
weaver_resolvertest suite to allow testing resolved schemaBTree*data structures for ordering.TODOs
requirement_level,sampling_relevant) unless they are actually overriden.