Skip to content

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

Merged
trask merged 5 commits into
open-telemetry:mainfrom
liurui-1:main
Mar 12, 2024
Merged

Fix the logic to get container.id resource attribute#10737
trask merged 5 commits into
open-telemetry:mainfrom
liurui-1:main

Conversation

@liurui-1

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

Copy link
Copy Markdown

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

liurui-1 commented Mar 4, 2024

Copy link
Copy Markdown
Author

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

@liurui-1

liurui-1 commented Mar 5, 2024

Copy link
Copy Markdown
Author

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

liurui-1 commented Mar 5, 2024

Copy link
Copy Markdown
Author

Hi @laurit , please help review again.

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

liurui-1 commented Mar 7, 2024

Copy link
Copy Markdown
Author

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
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