Skip to content

[CONTINT-1132] Send in-<inode> using the cgroup controller when container-id cannot be retrieved#232

Merged
AliDatadog merged 4 commits into
masterfrom
ali/inode-cid-resolver
Jan 16, 2024
Merged

[CONTINT-1132] Send in-<inode> using the cgroup controller when container-id cannot be retrieved#232
AliDatadog merged 4 commits into
masterfrom
ali/inode-cid-resolver

Conversation

@AliDatadog

@AliDatadog AliDatadog commented Jan 11, 2024

Copy link
Copy Markdown
Contributor

Implementation of DataDog/datadog-go#291 but in Java

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 depending on the cgroup version. 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:

0::
1:name=systemd:... // only in cgroupv1
Once the path is retrieved, we retrieve the inode of /sys/fs/cgroup + + where is the path retrieved previously.

I am not sure how to create a dummy app that uses this library.

Test

  • Set up a cgroupv2 environment. A kind cluster should be enough.
  • Deploy the agent 7.51.0+ with origin detection and dogstatsd enabled. Using helm:
  dogstatsd:
    port: 8125
    originDetection: true
    useHostPort: true
    useHostPID: true
    tagCardinality: high
  • Note that you will also need the env var:
    - name: "DD_DOGSTATSD_ORIGIN_DETECTION_CLIENT"
      value: "true"
  • Deploy a dummy app. I already made one with and without this change:
apiVersion: apps/v1
kind: Deployment
metadata:
  name: my-java-dsd-app
spec:
  replicas: 1
  selector:
    matchLabels:
      app: my-java-dsd-app
  template:
    metadata:
      labels:
        app: my-java-dsd-app
        admission.datadoghq.com/enabled: "false"
    spec:
      containers:
      - name: my-java-dsd-app
        image: docker.io/alidatadog/dummy-java-dsd-app:with-inode@sha256:a62e653e3c8e493611ba5841d3af94921edef0670776d0555fed9eecc2b50341 # use the tag without-inode for comparison
        imagePullPolicy: Always
        env:
        - name: DD_AGENT_HOST
          valueFrom:
            fieldRef:
              fieldPath: status.hostIP

@AliDatadog
AliDatadog requested a review from a team January 11, 2024 15:57
@AliDatadog
AliDatadog force-pushed the ali/inode-cid-resolver branch 4 times, most recently from 5950a63 to a773124 Compare January 12, 2024 10:30
@AliDatadog
AliDatadog force-pushed the ali/inode-cid-resolver branch from a773124 to 73bf9fb Compare January 12, 2024 11:21
}

/**
* Parses `path` (=/proc/self/cgroup) and returns the container ID if available.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't seem to match what the method is doing.

* the container ID.
*
*/
if ((containerID == null || containerID.equals("")) && !isHostCgroupNamespace(CGROUP_NS_PATH)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a comment explaining why we exclude isHostCgroupNamespace(CGROUP_NS_PATH) case?

Comment thread src/main/java/com/timgroup/statsd/CgroupReader.java
if (tokens.length != 3) {
continue;
}
if (CGROUPV1_BASE_CONTROLLER.equals(tokens[1]) || tokens[1].isEmpty()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (CGROUPV1_BASE_CONTROLLER.equals(tokens[1]) || tokens[1].isEmpty()) {
if (CGROUPV1_BASE_CONTROLLER.equals(tokens[1]) ||
CGROUPV2_BASE_CONTROLLER.equals(tokens[1])) {

// Docker
String docker = new StringBuilder()
.append("13:name=systemd:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860\n")
.append("12:pids:/docker/3726184226f5d3147c25fdeab5b60097e378e8a720503a5e19ecfdf29f869860\n")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't reformat the code that you didn't modify.

Comment thread src/main/java/com/timgroup/statsd/CgroupReader.java
@AliDatadog
AliDatadog force-pushed the ali/inode-cid-resolver branch from f5229f5 to c15f5f4 Compare January 16, 2024 14:05
@AliDatadog
AliDatadog merged commit edb30df into master Jan 16, 2024
@AliDatadog
AliDatadog deleted the ali/inode-cid-resolver branch January 16, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants