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

Fix the logic to get container.id resource attribute #10737

Merged
merged 5 commits into from
Mar 12, 2024
Merged

Fix the logic to get container.id resource attribute #10737

merged 5 commits into from
Mar 12, 2024

Conversation

liurui-1
Copy link

@liurui-1 liurui-1 commented Mar 2, 2024

As described in #10719, the container.id resource attribute returned by ContainerResource.java does not work for a lot of container environments. CgroupV1ContainerIdExtractor.java is OK. The problem is at CgroupV2ContainerIdExtractor.java. When the "/proc/self/cgroup" file does not include the container.id, with the "/proc/self/mountinfo" file, the CgroupV2ContainerIdExtractor.java usually returns a wrong value.

I refined the logic and added 4 new typical test scenarios besides the existing 2.

@liurui-1 liurui-1 requested a review from a team March 2, 2024 15:04
@liurui-1
Copy link
Author

liurui-1 commented Mar 4, 2024

Hi @laurit , I have revised the code according to your comments.

@liurui-1
Copy link
Author

liurui-1 commented Mar 5, 2024

Hi @laurit , I have revised the code according to your comments.

…ry/instrumentation/resources/CgroupV2ContainerIdExtractor.java

Co-authored-by: Lauri Tulmin <tulmin@gmail.com>
@liurui-1
Copy link
Author

liurui-1 commented Mar 5, 2024

Hi @laurit , please help review again.

@laurit laurit added this to the v2.2.0 milestone Mar 6, 2024
@liurui-1
Copy link
Author

liurui-1 commented Mar 7, 2024

Are there more steps to have this PR merged? Then when can we see it working in OTel Java auto-instrumentations?

@trask trask merged commit 93bb1fe into open-telemetry:main Mar 12, 2024
49 checks passed
github-actions bot pushed a commit that referenced this pull request May 10, 2024
Co-authored-by: Lauri Tulmin <tulmin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants