Create an entity modelling guide #2328
Conversation
| `model/{my_domain}/entities.yaml`: | ||
|
|
||
| ```yaml | ||
| groups: | ||
| - id: entity.my_entity | ||
| type: entity | ||
| stability: development | ||
| name: my_entity | ||
| brief: > | ||
| A description of my_entity here. | ||
| attributes: | ||
| - ref: some.attribute | ||
| role: identifying | ||
| - ref: some.other_attribute | ||
| role: descriptive | ||
| ... | ||
| ``` |
There was a problem hiding this comment.
great idea to include an example!
| where one or many of the named entities may be associated with the | ||
| metric. There is *no* requirement on disjointness due to how is-a | ||
| relationships are modelled. | ||
|
|
There was a problem hiding this comment.
should we mention that some entities (e.g. service) are implicitly associated with everything and don't need to be explicitly listed? Or should we encourage people to add all applicable ones?
There was a problem hiding this comment.
Great question.
I think we encourage people to add applicable ones. I also wonder if we SHOULD list service on many conventions we have today, to denote we expect SDKs to have service and data to be attached to it as a minimum thing. WDYT?
Also cc @open-telemetry/specs-entities-approvers for thoughts.
There was a problem hiding this comment.
let's try explicitly listing it and see if/how we like it
docs/non-normative/how-to-write-conventions/resource-and-entities.md
Outdated
Show resolved
Hide resolved
|
|
||
| - Default to introducing separate entities with a clear "is a" (or similar) relationship | ||
| - Extend an entity with new descriptive attributes if and only if the following is true: | ||
| - The extending entity cannot be associated with any telemetry by itself. |
There was a problem hiding this comment.
I don't understand this sentence. maybe an example would be helpful?
| - Default to introducing separate entities with a clear "is a" (or similar) relationship | ||
| - Extend an entity with new descriptive attributes if and only if the following is true: | ||
| - The extending entity cannot be associated with any telemetry by itself. | ||
| - The extending entity doesn’t have any other entities that logically can only be associated with it. |
There was a problem hiding this comment.
don't understand this one either
There was a problem hiding this comment.
This one I get. I can get rid of this when entity is a "signal" type, but effectively this is when you don't expect to report relationships for the entity. It's just a special case of the first rule, but needs to be clarified prior to entity-as-a-signal (phase 2 for Entities work) being defined in the specification.
docs/non-normative/how-to-write-conventions/resource-and-entities.md
Outdated
Show resolved
Hide resolved
docs/non-normative/how-to-write-conventions/resource-and-entities.md
Outdated
Show resolved
Hide resolved
docs/non-normative/how-to-write-conventions/resource-and-entities.md
Outdated
Show resolved
Hide resolved
| the SDK that is externally visible. An alternative is to have the SDK | ||
| provide a relationship between the service.instance.id and another entity | ||
| that is visible externally. Care should be taken when modelling Entities | ||
| to avoid this problem where possible. |
There was a problem hiding this comment.
can we add some reasoning/explanation that despite being anti-pattern, we make service.instance.id identifiable in https://github.com/open-telemetry/semantic-conventions/pull/2278/files#diff-3d85dfd25c44f916cd9a6c53adf7628b01d37ce93cd2fd5083567af3ee9a64ef due to its usefulness?
There was a problem hiding this comment.
This is describing that care that needs to be taken even when using service.instance.id to make sure you don't wind up with different identity depending on who's observing it. It's a bit of a major problem today, and manifests on kubernetes in this way:
- Reading container logs on a VM and trying to identify if those logs came from an OTEL service.
- Reading metrics via prometheus and trying to identify if those metrics came from an OTEL service.
- Accepting OTLP where the SDK did local discovery of OTEL service.
If you're not careful, you can wind up with three different service.instance.ids for the same SDK.
See:
- OpenTelemetry Operator trying to push down an ID that will match service.instance id
- Prometheus Mapping for server.instance.id: ](Should language SDK generate service.instance.id? opentelemetry-specification#3136)
So this guidance is actually helping users understand the "magic" for how service.instance.id can work, and why it's horribly confusing when it doesn't.
While we can allow more things like service.instance.id I don't think we need to or should unless there's no alternative (which there is not for service.instance.id). That's why the guidance in semconv is to have a system of record from which multiple observers can obtain the same identifier.
Regarding adding more rationale in this guide - I'll see what I can add.
Do you think what I typed here makes sense for the guide?
lmolkova
moved this from Untriaged
to Needs More Approval
in Semantic Conventions Triage
…ies.md Co-authored-by: Liudmila Molkova <[email protected]>
…ies.md Co-authored-by: Liudmila Molkova <[email protected]>
…ies.md Co-authored-by: Liudmila Molkova <[email protected]>
…ies.md Co-authored-by: Liudmila Molkova <[email protected]>
…ies.md Co-authored-by: Liudmila Molkova <[email protected]>
Fixes #2246
Changes
Please provide a brief description of the changes here.
Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.
Merge requirement checklist
[chore]