-
Notifications
You must be signed in to change notification settings - Fork 888
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
Add k8s.node
in semantic conventions
#1341
Conversation
It looks like |
I think it would be important to first clarify what we want to have in |
| Attribute | Type | Description | Examples | Required | | ||
|---|---|---|---|---| | ||
| `k8s.node.name` | string | The name of the node. | `opentelemetry-node` | No | | ||
| `k8s.node.ip` | string | The address of the node (dotted decimal for IPv4 or [RFC5952](https://tools.ietf.org/html/rfc5952) for IPv6. | `127.0.0.1` | No | |
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.
Is this the external IP or internal IP? (see https://kubernetes.io/docs/concepts/architecture/nodes/#addresses). It may be worth explicitly using k8s.node.external_ip
and k8s.node.internal_ip
instead.
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.
You're right; explicitly declare the type of address might better.
Besides, I found k8s may have multiple ExternalIP
and InternalIP
. Maybe we should use k8s.node.external_ips
and k8s.node.internal_ips
and use string[]
as type instead.
For instance: ['127.0.0.1', '127.0.0.2']
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.
@XSAM can you give me a link to a doc which says there may be multiple ExternalIP and InternalIP? I haven't been able to find this in Kubernetes docs.
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 haven't been able to find any documentation specifically about multiple internal or external, but the API definition definitely supports it. I did find an issue that references someone setting multiple IPs of the same type as well: kubernetes/kubernetes#61921
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.
We do not use arrays for ip address elsewhere in semantic conventions (e.g. net.peer.ip
) but using arrays here here looks justified.
I would prefer to avoid the complication and just have a single ip address, but I do not see a reasonable good behavior that uses just one of the ip addresses if the source of the data can have multiple.
- id: ip | ||
type: string | ||
brief: > | ||
The address of the node (dotted decimal for IPv4 or [RFC5952](https://tools.ietf.org/html/rfc5952) for IPv6. |
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.
Can you pls comment on what IP to prefer?
CC: @dashpole |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Closed by (#1390) |
Resolve #1334