-
Notifications
You must be signed in to change notification settings - Fork 290
[k8s] Define missing roles for entity attributes #3135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
c88d0d0 to
26fcdd3
Compare
26fcdd3 to
e40da0f
Compare
|
/cc @open-telemetry/semconv-k8s-approvers |
model/k8s/entities.yaml
Outdated
| - ref: k8s.container.name | ||
| role: identifying | ||
| - ref: k8s.container.restart_count | ||
| role: identifying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not certain we need to have this be identifying.
When grouping based on a k8s.container do I want to have restarts be considred separate containers?
It seems to me like you want to leverage a diffrent entity (like service.instance.id) to distinguish containers by restart. Or you need something like "Container instance" entity that would track this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When grouping based on a k8s.container do I want to have restarts be considred separate containers?
Hmm, I think you are right.
I remember the reason for having container's restart count both as an attribute and as a metric was to be able to use the attribute for identifying from which container instance a log was collected from. Specifically this is used for container's logs at https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/stanza/docs/operators/container.md#add-metadata-from-file-path (not sure if used in other places too).
I'll defer to @dmitryax here since he should remember more details.
It seems to me like you want to leverage a diffrent entity (like service.instance.id) to distinguish containers by restart. Or you need something like "Container instance" entity that would track this.
Probably for that purpose we could just use the container.id which k8sattributes processor can add if k8s.container.name is present? I can try and come-up with a configuration that verifies this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
e40da0f to
3f6451e
Compare
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Related to #3120
Changes
This PR adds roles for entity attributes that were missing this.
k8s.namespace.namecan be an identifying attribute within the scope of a K8s a cluster, hencek8s.namespace.uidis not defined at all nor used ink8sattributesprocessor of the Collector. I'd love to hear what people think about this and if we should consider introducing thek8s.namespace.uidas an identifying attribute for consistency across k8s resources/entities.Merge requirement checklist
[chore]