-
Notifications
You must be signed in to change notification settings - Fork 135
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
[CONTINT-3542] Send in-<inode>
using the cgroup controller when container-id cannot be retrieved
#291
Conversation
d649217
to
17adae9
Compare
…up to retrieve the cgroup node, read mountspoint+cgroupnode to retrieve the inode and store it as the new container id
f106b04
to
7918f9e
Compare
7918f9e
to
c10f227
Compare
in-<inode>
when container-id cannot be retrievedin-<inode>
when container-id cannot be retrieved on cgroupv2
statsd/container_test.go
Outdated
) | ||
|
||
// initContainerID initializes the container ID. | ||
func init() { | ||
initContainerID = dummyInitContainerId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was necessary because currently most of the tests will be broken depending on your environment. I noticed that a containerID was added to a lot of metrics when the expected result was hardcoded without containerID.
@@ -153,6 +222,9 @@ func initContainerID(userProvidedID string, cgroupFallback bool) { | |||
} else { | |||
containerID = readMountinfo(selfMountInfoPath) | |||
} | |||
if containerID != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we don't have a very reliable way to identify if we're inside a container. It means that even if the tracer is running on host, we'll send the inode of the cgroup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually thinking more about that. If you are in cgroup host namespace, it's useless to get inode because /proc/self/cgroup
is going yield a container id if present.
So I think it's fair to check for cgroup namesapce != host cgroup namespace to get inode
. This will avoid some work for the lib and the Agent.
You can see how it's implemented from this code:
https://github.com/DataDog/datadog-agent/blob/main/pkg/util/containers/metrics/system/collector_linux.go#L84-L91
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense, I applied it in the last commit
in-<inode>
when container-id cannot be retrieved on cgroupv2in-<inode>
using the cgroup controller when container-id cannot be retrieved
f98e4a1
to
ab13dd5
Compare
ab13dd5
to
79801ae
Compare
statsd/container_linux.go
Outdated
@@ -153,6 +215,9 @@ func initContainerID(userProvidedID string, cgroupFallback bool) { | |||
} else { | |||
containerID = readMountinfo(selfMountInfoPath) | |||
} | |||
if containerID != "" { | |||
containerID = getCgroupInode(mountsPath, cgroupPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as in trace lib.
in-<inode>
using the cgroup controller when container-id cannot be retrievedin-<inode>
using the cgroup controller when container-id cannot be retrieved
@@ -3,7 +3,7 @@ | |||
|
|||
package statsd | |||
|
|||
func initContainerID(userProvidedID string, cgroupFallback bool) { | |||
var initContainerID = func(userProvidedID string, cgroupFallback bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was necessary because initContainerID
would be used in every test, breaking them in some env
504e206
to
6acc2c0
Compare
This Pull Request implements the support of Origin Detection when the container-id is unavailable from inside the application container only with cgroupv2.
It first retrieves the cgroup node path by parsing
proc/self/cgroup
which is formatted differently in cgroup v1 and v2. See https://man7.org/linux/man-pages/man7/cgroups.7.html.The format is a list of
hierarchy-ID:controller-list:cgroup-path
similar to:Once the path is retrieved, we retrieve the inode of
/sys/fs/cgroup + <path>
where<path>
is the path retrieved previously.Not that:
os.Stat
which follows symlinks. Thus, we assume that the path is not a symlinkHow to test
docker.io/alidatadog/dummy-dsd-app:0.1.0@sha256:0eca30d71b4fc4ff667b17f46e8e0333712566b7cac2bbc65f60515b8b13e0cc
Dockerfile: