Skip to content
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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ New:

- Add `cloud.infrastructure_service` resource attribute
([#1112](https://github.com/open-telemetry/opentelemetry-specification/pull/1112))
- Add `k8s.node` resource attributes
([#1341](https://github.com/open-telemetry/opentelemetry-specification/pull/1341))

Updates:

Expand Down
16 changes: 16 additions & 0 deletions semantic_conventions/resource/k8s.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,22 @@ groups:
The name of the cluster.
examples: ['opentelemetry-cluster']

- id: k8s.node
prefix: k8s.node
brief: >
A Kubernetes Node.
attributes:
- id: name
type: string
brief: >
The name of the node.
examples: ['opentelemetry-node']
- id: ip
type: string
brief: >
The address of the node (dotted decimal for IPv4 or [RFC5952](https://tools.ietf.org/html/rfc5952) for IPv6.
Copy link
Member

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?

examples: ['127.0.0.1']

- id: k8s.namespace
prefix: k8s.namespace
brief: >
Expand Down
13 changes: 13 additions & 0 deletions specification/resource/semantic_conventions/k8s.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ Kubernetes object, but "name" is usually more user friendly so can be also set.
| `k8s.cluster.name` | string | The name of the cluster. | `opentelemetry-cluster` | No |
<!-- endsemconv -->

## Node

**type:** `k8s.node`

**Description:** A Kubernetes Node.

<!-- semconv k8s.node -->
| 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 |
Copy link
Member

@tigrannajaryan tigrannajaryan Jan 14, 2021

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.

Copy link
Member Author

@XSAM XSAM Jan 15, 2021

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_ipsand k8s.node.internal_ips and use string[] as type instead.

For instance: ['127.0.0.1', '127.0.0.2']

Copy link
Member

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.

Copy link
Contributor

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

Copy link
Member

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.

<!-- endsemconv -->

## Namespace

Namespaces provide a scope for names. Names of objects need to be unique within
Expand Down