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.pod.ip attribute #1160

Open
ChrsMark opened this issue Jun 17, 2024 · 9 comments
Open

Add k8s.pod.ip attribute #1160

ChrsMark opened this issue Jun 17, 2024 · 9 comments
Labels
area:k8s enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve

Comments

@ChrsMark
Copy link
Member

Area(s)

area:k8s

Is your change request related to a problem? Please describe.

The k8s.pod.ip is already used by the Collector for the association of Pod's signals with the respective Pod's metadata.
With open-telemetry/opentelemetry-collector-contrib#33440 the Collector will provide the option to explicitly expose this as an additional attribute as well.

Describe the solution you'd like

We would like to define a Semantic Convention to cover the k8s.pod.ip attribute.

Describe alternatives you've considered

No response

Additional context

One thing to consider here is that Pod's IP can change during Pod's existence. An interesting discussion/summary around this can be found at kubernetes/kubernetes#108281 (comment).

Having said this, the k8s.pod.ip will be mutable so it can't be a resource attribute similarly to the current status of a container discussed at #997 (comment).

However I wonder if this constraint is valid for all the Resource cases we want to cover in SemConv, and I think it is already violated in the k8s.pod.{label,annotation}.* case where a label or annotation can be overwritten.
(same for host's IPs, I guess)

@open-telemetry/specs-semconv-approvers I would love your thoughts here.

@lmolkova
Copy link
Contributor

@open-telemetry/semconv-k8s-approvers @jsuereth @tigrannajaryan please take a look

@TylerHelmuth
Copy link
Member

The name and location seem right to me. We've been using this field as a resource attribute for many many years and had no issues.

@mx-psi
Copy link
Member

mx-psi commented Jun 18, 2024

Regarding mutability we have a bunch other attributes that are mutable as you point out (host.ip being one of them). If we have host.ip, we surely can have k8s.pod.ip as well. I think the Entities SIG will have to figure mutability out, but until that happens I am in favor of using this as used in the Collector.

@tigrannajaryan
Copy link
Member

The concept of mutable attributes fits well with the upcoming concept of Entities. We plan to reflect this in the semconv in the future.

@joaopgrassi
Copy link
Member

joaopgrassi commented Jul 9, 2024

So, this is essentially blocked by the entity initiative?

@tigrannajaryan
Copy link
Member

We don't need to block it. We can add it as an experimental k8s Resource attribute. We may have to change it later depending on entities SIG work, but that's expected for experimental attributes.

Resource attributes are de-facto mutable today in the data model, from recipient's perspective. The SDK says they are immutable, but that is not guaranteed or enforced in any way between SDK restarts. The current reading of the spec should be "Resource attributes don't change during the lifetime of one SDK run".

@joaopgrassi
Copy link
Member

joaopgrassi commented Jul 9, 2024

An old, and similar issue #1160 #918. I closed that in favor of this one.

@ChrsMark
Copy link
Member Author

@joaopgrassi I think you meant #918 :), right?

@joaopgrassi
Copy link
Member

@joaopgrassi I think you meant #918 :), right?

Yes 😅 thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:k8s enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve
Projects
Status: No status
Status: Todo
Development

No branches or pull requests

6 participants