Add cgroup v2 support for container id detector #3508

XSAM · 2022-12-02T05:46:51Z

Resolves #3501

The new container id detector tries to resolve the container id from the cgroup v1 file, if successful, the result is used. If v1 fails, then the detector falls back to the cgroup v2 file.

codecov · 2022-12-02T05:49:29Z

Codecov Report

Merging #3508 (5eaaa5c) into main (69e44a3) will increase coverage by 0.0%.
The diff coverage is 84.6%.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #3508   +/-   ##
=====================================
  Coverage   78.0%   78.1%           
=====================================
  Files        164     166    +2     
  Lines      11815   11845   +30     
=====================================
+ Hits        9226    9252   +26     
- Misses      2393    2395    +2     
- Partials     196     198    +2

Impacted Files	Coverage Δ
sdk/resource/container_id_cgroup_v2.go	`75.0% <75.0%> (ø)`
sdk/resource/container.go	`86.6% <80.0%> (-5.7%)`	⬇️
sdk/resource/container_id_cgroup_v1.go	`100.0% <100.0%> (ø)`
sdk/metric/periodic_reader.go	`91.3% <0.0%> (+1.2%)`	⬆️

CHANGELOG.md

sdk/resource/container.go

sdk/resource/container_id_cgroup_v1.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

MadVikingGod

I found this doesn't work with podman. I tested it also with minikube docker, and found that I didn't get the proper containerID from V1, but the correct one from V2.

I made a gist of what I saw from /proc/self/cgroup and /proc/self/mointinfo https://gist.github.com/MadVikingGod/6ecba436717948c1a332c63f250f973e

MadVikingGod · 2022-12-02T17:21:27Z

sdk/resource/container_id_cgroup_v2.go

+
+const cgroupV2Path = "/proc/self/mountinfo"
+
+var cgroupV2ContainerIDRe = regexp.MustCompile(`.*/docker/containers/([0-9a-f]{64})/.*`)


This is very docker specific. I found with podman there is a different format for the container layers, and I am also checking containerd.

The same is used in Java here (https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/ca8e7b1385d95517f28103d834aba960a0baa477/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/CgroupV2ContainerIdExtractor.java#L27), so it would need to be fixed in both.

JavaScript is taking a different approach, I remember that we talked about containerd there and making it work:

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/ca8e7b1385d95517f28103d834aba960a0baa477/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/CgroupV2ContainerIdExtractor.java#L27

cc @breedx-splk , @abhee11

@svrnm Thanks for the heads-up. Looks like you pasted the same link twice, but it reads like the second one intended to link to a js discussion? Curious what the alt approach is...

@breedx-splk thanks for calling out the wrong link, here's the JS one: https://github.com/open-telemetry/opentelemetry-js-contrib/blob/f0a93685cfb43543b7ca577dd370d56576b49e3f/detectors/node/opentelemetry-resource-detector-container/src/detectors/ContainerDetector.ts#L54

MadVikingGod · 2022-12-02T17:23:09Z

sdk/resource/container_id_cgroup_v1.go

+
+const cgroupV1Path = "/proc/self/cgroup"
+
+var cgroupV1ContainerIDRe = regexp.MustCompile(`^.*/(?:.*-)?([0-9a-f]+)(?:\.|\s*$)`)


I don't think this is still the right way to capture this for V1.

When used with podman I'm capturing the userID of the pod, not a container or an empty string.

This is based on a conversation in [opentelemetry-go #3508](open-telemetry/opentelemetry-go#3508) and to be more consistent with [the js cgroupv2 parser impl](https://github.com/open-telemetry/opentelemetry-js-contrib/blob/f0a93685cfb43543b7ca577dd370d56576b49e3f/detectors/node/opentelemetry-resource-detector-container/src/detectors/ContainerDetector.ts#L68). Unsurprisingly, podman does not include the word `docker` in the `mountinfo` file. As a result, the container id parsing would fail from inside a podman container. This fixes that up to be more compatible.

…y-go into container-id-cgroup-v2

XSAM · 2022-12-15T09:27:35Z

I tested it also with minikube docker, and found that I didn't get the proper containerID from V1, but the correct one from V2.

I found a strange case where my minikube with Kubernetes v1.24.1 on Docker 20.10.17 does not expose the container id of the running container in the mountinfo file. Instead, it exposes the container id of the “pause” container.

1076 961 0:286 / / rw,relatime master:250 - overlay overlay rw,lowerdir=/var/lib/docker/overlay2/l/5D465AGLTKRGC5C6WYKS6HL4V2:/var/lib/docker/overlay2/l/TSSPM2QEYHI3WBSSGGKURHKT46:/var/lib/docker/overlay2/l/JR676YF42NBYUCOBKSN2ADZ4SN:/var/lib/docker/overlay2/l/MWHUAAOCGB6A6NDHB7XD3DO3BG:/var/lib/docker/overlay2/l/4ZYXIEUKFJJPC2JWW5MWLMTO5E:/var/lib/docker/overlay2/l/SUZ3KYKPSZ7N6DQJ2DH3Z6RSWD:/var/lib/docker/overlay2/l/45SS6JSP6LFRDWQO3X3QHLJ2YG,upperdir=/var/lib/docker/overlay2/376cc31f15c70b8881fd5df11c5e8fd95996dbb39c25476b969cba70cba8e8c8/diff,workdir=/var/lib/docker/overlay2/376cc31f15c70b8881fd5df11c5e8fd95996dbb39c25476b969cba70cba8e8c8/work
1077 1076 0:288 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw
1078 1076 0:289 / /dev rw,nosuid - tmpfs tmpfs rw,size=65536k,mode=755
1079 1078 0:290 / /dev/pts rw,nosuid,noexec,relatime - devpts devpts rw,gid=5,mode=620,ptmxmode=666
1080 1076 0:284 / /sys ro,nosuid,nodev,noexec,relatime - sysfs sysfs ro
1081 1080 0:33 / /sys/fs/cgroup ro,nosuid,nodev,noexec,relatime - cgroup2 cgroup rw
1082 1078 0:280 / /dev/mqueue rw,nosuid,nodev,noexec,relatime - mqueue mqueue rw
1083 1078 254:1 /docker/volumes/minikube/_data/lib/kubelet/pods/3c0722a4-41c7-4901-9163-f4b57961fe77/containers/nginx/823ef551 /dev/termination-log rw,relatime - ext4 /dev/vda1 rw
1084 1076 254:1 /docker/volumes/minikube/_data/lib/docker/containers/f9f9c232fdd59b55d94aa546f40c071a21c224677d792d323b86ca94b868a1ef/resolv.conf /etc/resolv.conf rw,relatime - ext4 /dev/vda1 rw
1085 1076 254:1 /docker/volumes/minikube/_data/lib/docker/containers/f9f9c232fdd59b55d94aa546f40c071a21c224677d792d323b86ca94b868a1ef/hostname /etc/hostname rw,relatime - ext4 /dev/vda1 rw
1086 1076 254:1 /docker/volumes/minikube/_data/lib/kubelet/pods/3c0722a4-41c7-4901-9163-f4b57961fe77/etc-hosts /etc/hosts rw,relatime - ext4 /dev/vda1 rw
1087 1078 0:279 / /dev/shm rw,nosuid,nodev,noexec,relatime - tmpfs shm rw,size=65536k
1088 1076 0:276 / /run/secrets/kubernetes.io/serviceaccount ro,relatime - tmpfs tmpfs rw,size=10189984k
962 1077 0:288 /bus /proc/bus ro,nosuid,nodev,noexec,relatime - proc proc rw
963 1077 0:288 /fs /proc/fs ro,nosuid,nodev,noexec,relatime - proc proc rw
964 1077 0:288 /irq /proc/irq ro,nosuid,nodev,noexec,relatime - proc proc rw
965 1077 0:288 /sys /proc/sys ro,nosuid,nodev,noexec,relatime - proc proc rw
966 1077 0:288 /sysrq-trigger /proc/sysrq-trigger ro,nosuid,nodev,noexec,relatime - proc proc rw
967 1077 0:289 /null /proc/kcore rw,nosuid - tmpfs tmpfs rw,size=65536k,mode=755
968 1077 0:289 /null /proc/keys rw,nosuid - tmpfs tmpfs rw,size=65536k,mode=755
969 1077 0:289 /null /proc/timer_list rw,nosuid - tmpfs tmpfs rw,size=65536k,mode=755
1002 1077 0:289 /null /proc/sched_debug rw,nosuid - tmpfs tmpfs rw,size=65536k,mode=755
1003 1080 0:291 / /sys/firmware ro,relatime - tmpfs tmpfs ro

In this case, f9f9c232fdd59b55d94aa546f40c071a21c224677d792d323b86ca94b868a1ef is not the container id of the running container. It belongs to the “pause” container. The actual container id is dbc9afa6e436346503ed45b871cc993528f1caf98adea47fb3f88186d05c28fb, which is not present in the file.

@MadVikingGod What was your environment to get those mountinfo with container id?

MadVikingGod · 2023-01-02T20:36:47Z

@MadVikingGod What was your environment to get those mountinfo with container id?

I started minikube with a different --container-runtime=[docker, cri-o, containerd].

minikube start -p cri-o --container-runtime=cri-o
...
minikube -p cri-o kubectl ...

From what I've seen I don't know if there is a reliable way to get the containerID from within the context of the running container. It does seem like docker might do it this way, but that doesn't look like it's guaranteed between versions, and different runtimes do things differently.

Add cgroup v2 support for container id detector

4e26e1a

Update CHANGELOG

aa3640c

XSAM marked this pull request as ready for review December 2, 2022 05:54

XSAM requested review from jmacd, MrAlias, Aneurysm9, evantorrie, dashpole, MadVikingGod, pellared, hanyuancheung and dmathieu as code owners December 2, 2022 05:54

dmathieu approved these changes Dec 2, 2022

View reviewed changes

MrAlias approved these changes Dec 2, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

sdk/resource/container.go Outdated Show resolved Hide resolved

sdk/resource/container_id_cgroup_v1.go Outdated Show resolved Hide resolved

XSAM and others added 3 commits December 2, 2022 23:51

Update CHANGELOG.md

82e8099

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

Apply suggestions from code review

f5ce8d2

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

Change id to ID

a245a1b

MadVikingGod requested changes Dec 2, 2022

View reviewed changes

Merge branch 'main' into container-id-cgroup-v2

cef565e

hanyuancheung approved these changes Dec 6, 2022

View reviewed changes

breedx-splk mentioned this pull request Dec 6, 2022

More flexible cgroupv2 container id parsing (and podman support) open-telemetry/opentelemetry-java-instrumentation#7361

Merged

XSAM added 5 commits December 13, 2022 16:25

Fix an issue where resolving the wrong container id on the podman

dd75a26

Fit more container ID cases

840d2ac

Merge branch 'container-id-cgroup-v2' of github.com:XSAM/opentelemetr…

5a73738

…y-go into container-id-cgroup-v2

Add more test cases

1f5bf29

Merge branch 'main' into container-id-cgroup-v2

f34b099

Merge branch 'main' into container-id-cgroup-v2

5eaaa5c

svrnm mentioned this pull request Jan 2, 2023

Add resource detection to checkout service open-telemetry/opentelemetry-demo#662

Merged

2 tasks

pellared mentioned this pull request Jan 2, 2023

Added CGroupv2 support into Docker Extensions open-telemetry/opentelemetry-dotnet-contrib#839

Merged

svrnm mentioned this pull request Jan 19, 2023

provide a portable mechanism for processes within container to obtain their image and container ids opencontainers/runtime-spec#1105

Open

breedx-splk mentioned this pull request Apr 3, 2023

ContainerResource extract container.id in Host env open-telemetry/opentelemetry-java-instrumentation#7437

Closed

calohmn mentioned this pull request Sep 3, 2023

Topics hono.command_internal.* are not being cleaned up on Docker versions >20.10 eclipse-hono/hono#3537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cgroup v2 support for container id detector #3508

Add cgroup v2 support for container id detector #3508

XSAM commented Dec 2, 2022

codecov bot commented Dec 2, 2022 •

edited

MadVikingGod left a comment

MadVikingGod Dec 2, 2022

svrnm Dec 5, 2022

svrnm Dec 5, 2022

breedx-splk Dec 5, 2022

svrnm Dec 6, 2022

MadVikingGod Dec 2, 2022

XSAM commented Dec 15, 2022

MadVikingGod commented Jan 2, 2023


		const cgroupV2Path = "/proc/self/mountinfo"

		var cgroupV2ContainerIDRe = regexp.MustCompile(`./docker/containers/([0-9a-f]{64})/.`)


		const cgroupV1Path = "/proc/self/cgroup"

		var cgroupV1ContainerIDRe = regexp.MustCompile(`^./(?:.-)?([0-9a-f]+)(?:\.\|\s*$)`)

Add cgroup v2 support for container id detector #3508

Are you sure you want to change the base?

Add cgroup v2 support for container id detector #3508

Conversation

XSAM commented Dec 2, 2022

codecov bot commented Dec 2, 2022 • edited

Codecov Report

MadVikingGod left a comment

Choose a reason for hiding this comment

MadVikingGod Dec 2, 2022

Choose a reason for hiding this comment

svrnm Dec 5, 2022

Choose a reason for hiding this comment

svrnm Dec 5, 2022

Choose a reason for hiding this comment

breedx-splk Dec 5, 2022

Choose a reason for hiding this comment

svrnm Dec 6, 2022

Choose a reason for hiding this comment

MadVikingGod Dec 2, 2022

Choose a reason for hiding this comment

XSAM commented Dec 15, 2022

MadVikingGod commented Jan 2, 2023

codecov bot commented Dec 2, 2022 •

edited