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 (linux).process.cgroup attribute #1357

Open
rogercoll opened this issue Aug 20, 2024 · 4 comments
Open

Add (linux).process.cgroup attribute #1357

rogercoll opened this issue Aug 20, 2024 · 4 comments
Assignees
Labels
area:security area:system enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage

Comments

@rogercoll
Copy link
Contributor

Area(s)

area:system

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

The hostmetrics receiver is currently reporting the process.cgroup attribute: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/hostmetricsreceiver/internal/scraper/processscraper/metadata.yaml#L49

This attribute is really helpful as it can be used to extract containerization information like the K8s pod UID and/or the container ID which the process is running on.

Describe the solution you'd like

Standardize the process.cgroup attribute.

Describe alternatives you've considered

As cgroups being a Linux only technology, would it make sense to add the attribute under the linux namespace (e.g linux.process.cgroup)?

Additional context

No response

@rogercoll rogercoll added enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage labels Aug 20, 2024
@braydonk
Copy link
Contributor

I'm in favour of this attribute being under the linux namespace.

@trisch-me
Copy link
Contributor

Hey @rogercoll, don't you think process will be better option for it? I know it's specific to linux, but process is common for all OS. I think the important namespace here is process and not the linux

@mjwolf what do you think?

@mjwolf
Copy link
Contributor

mjwolf commented Sep 9, 2024

I think this would be better under process as well. There are already OS specific attributes in process; process.owner is Windows specific while process.user.id and process.group.id. #1329 will also add another one with gnu.build_id. So I don't see a problem with adding process.cgroup, and it seems like a more natural location to me.

@rogercoll
Copy link
Contributor Author

don't you think process will be better option for it?

I don't have a strong opinion about it, given that we already have OS specific attributes without the OS namespace it might make sense to have a unified approach (without OS).

We discussed this naming issue during today's system SIG and we notice that some attributes contain the OS while others don't, I think we should agree on the structure and fix these nuances. #1403

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:security area:system enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage
Development

No branches or pull requests

5 participants