Live check entity associations#1426
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1426 +/- ##
=======================================
+ Coverage 82.2% 82.3% +0.1%
=======================================
Files 125 125
Lines 10225 10305 +80
=======================================
+ Hits 8406 8484 +78
- Misses 1819 1821 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| value: "conditionally_required_attribute_not_present" | ||
| brief: "A conditionally required attribute is absent from the sample" | ||
| stability: development | ||
| # Entity requirement-level findings (resource attributes checked against entity associations) |
| - id: entity_conditionally_required_attribute_not_present | ||
| value: "entity_conditionally_required_attribute_not_present" | ||
| brief: "A conditionally required entity attribute is absent from the resource" | ||
| stability: development |
There was a problem hiding this comment.
Side question - How are planning to do stability/release for these? You can answer offline.
| Some(VersionedSignal::Event(e)) => &e.entity_associations, | ||
| _ => &[], | ||
| }; | ||
| for entity_type in entity_associations { |
There was a problem hiding this comment.
So we only require one of the entities in entity associations to match.
I think as a first draft this is good - as we don't have any entities with multiple associations yet.
However the model allows any-of matching, so e.g. we wouldn't want to emit violations of one entity association if another one was "clean" and we don't track entity type in OTLP yet (It's there, but it's not used).
I think this is OK for now - but we should document this limitation in weaver or somehow denote it in the error message.
There was a problem hiding this comment.
Oh I see. I did not realise that. I just assumed the entities were ANDED. So does "clean" mean that all the required attributes from an entity are satisfied? We could have logic:
Given entities A,B,C - if A is clean, report findings for non-required attributes on A only. If A is dirty, look at B and so on. If all are dirty report findings for all.
Would that work? Must there be at least one required attribute on an entity?
There was a problem hiding this comment.
So entities have identity attribute and and description attributes.
You can detect presense if all the identity attribute are there. implicilty (in v2) all identity attributes are required.
For description - SOME may be required (although that hasn't happened yet), bu tshes can be warnings.
So for your logic - I'd use the identity attribute to determine if "A" is present, e.g. and if you find none, warn that you didn't find an identity for one of the given entities.
Also - at least for semconv - we're keeping identity attributes distinct so there should be no overlap where one attribute could be the id of two entities in semconv, you can decide if we allow that in weaver - I'm on the fence myself. Today it's basically allowed because it's not disallowed (we'd have to detect and error)
Fixes: #1297
Adds entity live-checks.
